Bug 1624542
| Summary: | Fix use-after-free in rpc.statd monitor list when insertion fails for existing entry | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 7 | Reporter: | Frank Sorenson <fsorenso> | ||||||
| Component: | nfs-utils | Assignee: | Steve Dickson <steved> | ||||||
| Status: | CLOSED ERRATA | QA Contact: | Yongcheng Yang <yoyang> | ||||||
| Severity: | unspecified | Docs Contact: | |||||||
| Priority: | medium | ||||||||
| Version: | 7.5 | CC: | dwysocha, steved, swhiteho, xzhou | ||||||
| Target Milestone: | rc | Keywords: | Patch, Reproducer | ||||||
| Target Release: | --- | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Whiteboard: | |||||||||
| Fixed In Version: | nfs-utils-1.3.0-0.62.el7 | Doc Type: | If docs needed, set a value | ||||||
| Doc Text: | Story Points: | --- | |||||||
| Clone Of: | Environment: | ||||||||
| Last Closed: | 2019-08-06 13:11:50 UTC | Type: | Bug | ||||||
| Regression: | --- | Mount Type: | --- | ||||||
| Documentation: | --- | CRM: | |||||||
| Verified Versions: | Category: | --- | |||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||
| Embargoed: | |||||||||
| Bug Depends On: | |||||||||
| Bug Blocks: | 1577173 | ||||||||
| Attachments: |
|
||||||||
Created attachment 1480593 [details] reproducer for bz1624542 reproducer; requires nsm_client test program from nfs-utils source committed to upstream nfs-utils
commit 86604e2bd536ea48832dd0bf3d95b15de4de2733 (HEAD -> master, origin/master, origin/HEAD)
Author: Steve Dickson <steved>
Date: 2018-09-06 10:22:11 -0400
statd: fix use-after-free in monitor list if insertion fails
If nsm_insert_monitored_host() fails while saving the record to
stable storage, we can't just assume the entry was new. Existing
records must be removed from the list before being freed.
Reviewed-by: Chuck Lever <chuck.lever>
Signed-off-by: Frank Sorenson <sorenson>
Signed-off-by: Steve Dickson <steved>
Have verified in nfs-utils-1.3.0-0.63.el7 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ https://beaker.engineering.redhat.com/recipes/6693240 ----------------------------------------------------- [08:19:11 root@ ~~]# ./repro.sh doing initial 'monitor' calls Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.1 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.2 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.3 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.4 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.5 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.6 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.7 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.8 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.9 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.10 SM_MON request successful, state: 3 changing directory ownership to root attempting to 'monitor' with new cookie RPC:0 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.10 SM_MON request failed, state: -1 RPC:0 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.9 SM_MON request failed, state: -1 RPC:0 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.8 SM_MON request failed, state: -1 RPC:0 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.7 SM_MON request failed, state: -1 RPC:0 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.6 SM_MON request failed, state: -1 RPC:0 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.5 SM_MON request failed, state: -1 RPC:0 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.4 SM_MON request failed, state: -1 RPC:0 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.3 SM_MON request failed, state: -1 RPC:0 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.2 SM_MON request failed, state: -1 RPC:0 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.1 SM_MON request failed, state: -1 RPC:0 Calling 127.0.0.1 (as rhel-7.6.redhat.com) to monitor 192.168.123.1 SM_MON request failed, state: -1 rpc.statd is still running ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PASS (Expected 0, got 0) ---------------------------------------------------- Compared with previous nfs-utils-1.3.0-0.61.el7 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ https://beaker.engineering.redhat.com/recipes/6693242 ----------------------------------------------------- [08:38:52 root@ ~~]# ./repro.sh doing initial 'monitor' calls Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.1 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.2 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.3 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.4 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.5 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.6 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.7 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.8 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.9 SM_MON request successful, state: 3 Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.10 SM_MON request successful, state: 3 changing directory ownership to root attempting to 'monitor' with new cookie RPC:0 Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.10 SM_MON request failed, state: -1 RPC:0 Calling 127.0.0.1 (as rhel-7.7.redhat.com) to monitor 192.168.123.9 sm_mon_1: RPC: Unable to receive; errno = Connection refused rpc.statd has died ^^^^^^^^^^^^^^^^^^^ FAIL - Should not crash the rpc.statd (Expected 0, got 1) -------------------------------------------------------------------------- Moving to VERIFIED for now. Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2019:2268 |
Created attachment 1480181 [details] patch to fix statd use-after-free Description of problem: If nsm_insert_monitored_host() fails while saving the record to stable storage, we currently just free the entry and return with error. This can lead to a use-after-free, causing rpc.statd to crash. We can't just assume the entry was new. Existing records must be removed from the list before being freed. Version-Release number of selected component (if applicable): all How reproducible: easy Steps to Reproduce: using the test program nsm_client: # /tmp/nsm_client mon client1 1 # /tmp/nsm_client mon client2 1 chown -R root:root /var/lib/nfs/statd/sm{,.bak} # /tmp/nsm_client mon client1 2 Actual results: use-after-free; potential crash of statd Expected results: entry is removed from the list Additional info: the entry is allocated in nlist_new: notify_list * nlist_new(char *my_name, char *mon_name, int state) { notify_list *new; new = (notify_list *) xmalloc(sizeof(notify_list)); the entry is freed after nsm_insert_monitored_host() fails: sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp) ... /* * Now, Create file on stable storage for host, first deleting any * existing records on file. */ nsm_delete_monitored_host(dnsname, mon_name, my_name, 0); if (!nsm_insert_monitored_host(dnsname, (struct sockaddr *)(char *)&my_addr, argp)) { nlist_free(NULL, clnt); goto failure; } however, note that an existing entry will not be removed from the list, just freed. this memory may be accessed again while walking the list: sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp) ... clnt = rtnl; while ((clnt = nlist_gethost(clnt, mon_name, 0))) { if (statd_matchhostname(NL_MY_NAME(clnt), my_name) && NL_MY_PROC(clnt) == id->my_proc && NL_MY_PROG(clnt) == id->my_prog && NL_MY_VERS(clnt) == id->my_vers) { if (memcmp(NL_PRIV(clnt), argp->priv, SM_PRIV_SIZE)) {