On 17/01/17(Tue) 00:24, Alexander Bluhm wrote:
> On Mon, Jan 16, 2017 at 08:36:46PM +0100, [email protected] wrote:
> > kernel: protection fault trap, code=0
> > Stopped at fd_getfile+0x20: testb $0x2,mptramp_gdt32_desc+0x1e(%r
> > ax)
> > ddb{3}> fd_getfile() at fd_getfile+0x20
> > sys_fstat() at sys_fstat+0x43
> > syscall() at syscall+0x27b
> 
> It crashes in fd_getfile() FILE_IS_USABLE(fp) as fdp->fd_ofiles has
> been freed.

Are you sure?  The faulting instruction is:

/sys/kern/kern_descrip.c:190
        80:       f6 40 40 02             testb  $0x2,0x40(%rax)
        84:       75 e7                   jne    6d <fd_getfile+0xd>

So %rax contains an incorrect value which is not NULL, are you
suggesting that it is garbage due to free(9) poisoning the memory? 

If that's the case, the easiest fix in would be to do the allocations
upfront.  An alternative solution discussed here at a2k17 would be to
use a SRP to guarantee that fd_ofiles is not freed while another CPU is
still referencing it.  That would also help for MP work.

Or could it be another race that your lock is preventing?

What about the diff below?  Cheeky does it help?

Index: kern/kern_descrip.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.136
diff -u -p -r1.136 kern_descrip.c
--- kern/kern_descrip.c 24 Sep 2016 18:39:17 -0000      1.136
+++ kern/kern_descrip.c 23 Jan 2017 10:40:53 -0000
@@ -835,6 +835,16 @@ fdexpand(struct proc *p)
                nfiles = 2 * fdp->fd_nfiles;
 
        newofile = mallocarray(nfiles, OFILESIZE, M_FILEDESC, M_WAITOK);
+       /*
+        * Allocate all required chunks before calling free(9) to make
+        * sure that ``fd_ofiles'' stays valid if we go to sleep.
+        */
+       if (NDHISLOTS(nfiles) > NDHISLOTS(fdp->fd_nfiles)) {
+               newhimap = mallocarray(NDHISLOTS(nfiles), sizeof(u_int),
+                   M_FILEDESC, M_WAITOK);
+               newlomap = mallocarray(NDLOSLOTS(nfiles), sizeof(u_int),
+                   M_FILEDESC, M_WAITOK);
+       }
        newofileflags = (char *) &newofile[nfiles];
 
        /*
@@ -853,11 +863,6 @@ fdexpand(struct proc *p)
                free(fdp->fd_ofiles, M_FILEDESC, fdp->fd_nfiles * OFILESIZE);
 
        if (NDHISLOTS(nfiles) > NDHISLOTS(fdp->fd_nfiles)) {
-               newhimap = mallocarray(NDHISLOTS(nfiles), sizeof(u_int),
-                   M_FILEDESC, M_WAITOK);
-               newlomap = mallocarray(NDLOSLOTS(nfiles), sizeof(u_int),
-                   M_FILEDESC, M_WAITOK);
-
                copylen = NDHISLOTS(fdp->fd_nfiles) * sizeof(u_int);
                memcpy(newhimap, fdp->fd_himap, copylen);
                memset((char *)newhimap + copylen, 0,

Reply via email to