Re: Potential free uninitialized pointer in kern_ktrace.c

[Philip Guenther]

On Sat, Aug 1, 2015 at 12:05 PM, Doug Hogan <d...@acyclic.org> wrote:
> On Sat, Aug 01, 2015 at 07:31:58PM +0100, Mark Latimer wrote:
>> reading through the compiler warnings I believe there is a potential issue
>> in /usr/src/sys/kern/kern_ktrace.c At first glance it appears to free
>> an uninitialized pointer memp.
>
> I agree.

Since my error was moving code across a goto, I'm inclined to kill the
goto completely, like this:


--- kern_ktrace.c       19 Jul 2015 04:45:25 -0000      1.74
+++ kern_ktrace.c       1 Aug 2015 18:51:10 -0000
@@ -361,21 +361,17 @@ ktruser(struct proc *p, const char *id,
        ktrinitheader(&kth, p, KTR_USER);
        memset(ktp.ktr_id, 0, KTR_USER_MAXIDLEN);
        error = copyinstr(id, ktp.ktr_id, KTR_USER_MAXIDLEN, NULL);
-       if (error)
-               goto out;
-
-       if (len > sizeof(stkbuf))
-               memp = malloc(len, M_TEMP, M_WAITOK);
-       else
-               memp = stkbuf;
-       error = copyin(addr, memp, len);
-       if (error)
-               goto out;
-
-       ktrwrite2(p, &kth, &ktp, sizeof(ktp), memp, len);
-out:
-       if (memp != stkbuf)
-               free(memp, M_TEMP, len);
+       if (error == 0) {
+               if (len > sizeof(stkbuf))
+                       memp = malloc(len, M_TEMP, M_WAITOK);
+               else
+                       memp = stkbuf;
+               error = copyin(addr, memp, len);
+               if (error == 0)
+                       ktrwrite2(p, &kth, &ktp, sizeof(ktp), memp, len);
+               if (memp != stkbuf)
+                       free(memp, M_TEMP, len);
+       }
        atomic_clearbits_int(&p->p_flag, P_INKTR);
        return (error);
 }