Bug 238014

Summary: Potential NULL pointer dereference in function ata_dev_advinfo and nvme_dev_advinfo
Product: Base System Reporter: Young <yangx92>
Component: kernAssignee: freebsd-bugs (Nobody) <bugs>
Status: New ---    
Severity: Affects Many People CC: splavecl
Priority: --- Keywords: patch
Version: CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Proposed patch none

Description Young 2019-05-21 07:42:31 UTC
Created attachment 204502 [details]
Proposed patch

There are two NULL pointer vulnerabilities in functions named ata_dev_advinfo (sys/cam/ata/ata_xpt.c) and nvme_dev_advinfo (sys/cam/nvme/nvme_xpt.c).

We take function ata_dev_advinfo for example.

                if (cdai->flags & CDAI_FLAG_STORE) {
                        if (device->physpath != NULL)
                                free(device->physpath, M_CAMXPT);
                        device->physpath_len = cdai->bufsiz;
                        /* Clear existing buffer if zero length */
                        if (cdai->bufsiz == 0)
                                break;
                        device->physpath = malloc(cdai->bufsiz, M_CAMXPT, M_NOWAIT);
                        if (device->physpath == NULL) {
                                start_ccb->ccb_h.status = CAM_REQ_ABORTED;
                                return;
                        }
                        memcpy(device->physpath, cdai->buf, cdai->bufsiz);


if the physical path is being stored and there is a malloc failure (malloc(9) is called with M_NOWAIT), we could wind up in a situation where the device's physpath_len is set to the length the user provided, but the physpath itself is NULL.

If another context then comes in to fetch the physical path value, we would wind up trying to memcpy a NULL pointer into the caller's buffer.

So, set the physpath_len to 0 when we free the physpath on entry into the store case for the physical path.  Reset the length to a non-zero value only after we've successfully malloced a buffer to hold it.

The attachment is the proposed patch.