kern/138657: Kernel memory corruption, kernel panic as a result

[Archer]

>Number:         138657
>Category:       kern
>Synopsis:       Kernel memory corruption, kernel panic as a result
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Sep 09 09:10:03 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator:     Archer
>Release:        FreeBSD 8.0-BETA4
>Organization:
n/a
>Environment:
FreeBSD serv 8.0-BETA4 FreeBSD 8.0-BETA4 #1: Sat Sep 8 19:20:53 MSD 2009 root at serv:/usr/obj/usr/src/sys/MYKERN i386
>Description:
Lets take a look at this code in ufs_vnops.c file:
refcount_init(&ucred.cr_ref, 1);
ucred.cr_uid = ip->i_uid;
ucred.cr_ngroups = 1;
ucred.cr_groups[0] = dp->i_gid;<---problem here
ucp = &ucred;

ucred structure variable is used here. It's a local structure so it's allocated on stack. Now lets take a look at the structure definition:
struct ucred {
--skipped...
gid_t *cr_groups;
--skipped...
};

As we see we've got a pointer. But this pointer is not initialized at the point of usage. So it contains some arbitrary value from the stack. And then that memory gets overwritten. Lets take a look at the assembly listing:
mov edx, [esi+6Ch]
mov eax, [ebp+var_140]<---here we read some arbitrary pointer from the stack
mov [eax], edx<---and here we smash the data at our arbitrary pointer

In my case some memory around ufs_lookup was overwritten.
>How-To-Repeat:
Enable QUOTA and SUIDDIR at the kernel options, set them on some partition and try to create a directory there.
For me worked tar -xvjf phpMyAdmin-3.2.1-all-languages.tar.bz2 which also calls mkdir when extracting files.

>Fix:
In FreeBSD 7.1 ucred structure was defined with cr_groups array instead or pointer. So at some places in 8.0 version this was fixed by creating local gid_t and assigning it's address to the pointer. And some places (and described above is one of them was missed for unknown reason). 
I suppose we should allocate memory for our pointer before using it so recklessly. It's also possible to create a local gid_t and assigning it's address to the pointer (this is done a bit lower at ufs_vnops.c file). And I suggest to review the code that works with this pointer, the same mistake may happen elsewhere.

>Release-Note:
>Audit-Trail:
>Unformatted: