Martin Pieuchot wrote:
> 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?
In that case, I think it's better to move the free down. This is more
idiomatic, free the old value and immediately replace.
Index: 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_descrip.c 24 Sep 2016 18:39:17 -0000 1.136
+++ kern_descrip.c 23 Jan 2017 22:02:07 -0000
@@ -849,9 +849,6 @@ fdexpand(struct proc *p)
memcpy(newofileflags, fdp->fd_ofileflags, copylen);
memset(newofileflags + copylen, 0, nfiles * sizeof(char) - copylen);
- if (fdp->fd_nfiles > NDFILE)
- 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);
@@ -877,6 +874,9 @@ fdexpand(struct proc *p)
fdp->fd_himap = newhimap;
fdp->fd_lomap = newlomap;
}
+
+ if (fdp->fd_nfiles > NDFILE)
+ free(fdp->fd_ofiles, M_FILEDESC, fdp->fd_nfiles * OFILESIZE);
fdp->fd_ofiles = newofile;
fdp->fd_ofileflags = newofileflags;
fdp->fd_nfiles = nfiles;
