2007-08-02 09:04:20

by Walter Stefan

[permalink] [raw]
Subject: Re: rpc.mountd crashes when extensively using netgroups

Steve Dickson wrote:
> Stefan Walter wrote:
>>
>> We do this on a much larger scale though. The bug we ran into is
>> in line 96 in utils/mountd/auth.c. The strcpy can corrupt
>> memory when it copies the string returned by client_compose() to
>> my_client.m_hostname which has a fixed size of 1024 bytes. For our
>> example above, client_compose() returns "@joe,@jane"
>> for any machine in the offices_1 netgroup. Unfortunately we have
>> a machine to which roughly 150 netgroups like @joe or @jane
>> export to and client_compose() returns a string over 1300 bytes
>> long and rpc.mountd nicely segfaults.
>>
>> To prevent the crash is of course trivial: Inserting a simple
>> 'if (strlen(n) > 1024) return NULL;' before line 96 does the job.
> Does the attached patch help?
>
rpc.mountd does not crash anymore but I get a 'permission denied' when
trying
to mount a share. Doing an 'strace rpc.mountd -F' reveals:

...
open("/proc/net/rpc/auth.unix.ip/channel", O_WRONLY|O_CREAT|O_TRUNC,
0666) = 9
fstat64(9, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
0) = 0xb7f41000
time(NULL) = 1186041882
write(9, "nfsd 129.132.10.33 1186043682 @a"..., 1024) = -1 EINVAL
(Invalid argument)
write(9, "\n", 1) = -1 EINVAL (Invalid argument)
close(9) = 0
munmap(0xb7f41000, 4096) = 0
open("/proc/net/rpc/nfsd.export/channel", O_WRONLY|O_CREAT|O_TRUNC,
0666) = 9
fstat64(9, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
0) = 0xb7f41000
write(9, "@anbuehle,@anhorni,@antoinet,@ap"..., 1024) = -1 EINVAL
(Invalid argument)
time(NULL) = 1186041882
write(9, "/export/groups/grossm/h1/home/gr"..., 68) = -1 ENOENT (No such
file or directory)
close(9) = 0
munmap(0xb7f41000, 4096) = 0
open("/proc/fs/nfsd/filehandle", O_RDWR) = 9
fstat64(9, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
0) = 0xb7f41000
write(9, "@anbuehle,@anhorni,@antoinet,@ap"..., 1066) = -1 EPERM
(Operation not permitted)
...

>>
>> Our ultimate goal is to get Red Hat fix the code in nfs-utils 1.0.6
>> that is used in RHEL4. A first step would be to get a suitable fix in
>> the current nfs-utils.
> Please open up bugs on all three of these issues and
> we'll see what can done...
>

Just did that (request IDs 1765949, 1765938 and 1765930 on sourceforge).

- Stefan


2007-08-03 02:28:24

by Satyam Sharma

[permalink] [raw]
Subject: Re: rpc.mountd crashes when extensively using netgroups

Hi,


On Thu, 2 Aug 2007, Stefan Walter wrote:

> Steve Dickson wrote:
> > Stefan Walter wrote:
> >>
> >> We do this on a much larger scale though. The bug we ran into is
> >> in line 96 in utils/mountd/auth.c. The strcpy can corrupt
> >> memory when it copies the string returned by client_compose() to
> >> my_client.m_hostname which has a fixed size of 1024 bytes. For our
> >> example above, client_compose() returns "@joe,@jane"
> >> for any machine in the offices_1 netgroup. Unfortunately we have
> >> a machine to which roughly 150 netgroups like @joe or @jane
>> export to and client_compose() returns a string over 1300 bytes
> >> long and rpc.mountd nicely segfaults.
> >>
> >> To prevent the crash is of course trivial: Inserting a simple
> >> 'if (strlen(n) > 1024) return NULL;' before line 96 does the job.
> > Does the attached patch help?
> >
> rpc.mountd does not crash anymore but I get a 'permission denied' when
> trying
> to mount a share. Doing an 'strace rpc.mountd -F' reveals:
>
> ...
> open("/proc/net/rpc/auth.unix.ip/channel", O_WRONLY|O_CREAT|O_TRUNC,
> 0666) = 9
> fstat64(9, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
> mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
> 0) = 0xb7f41000
> time(NULL) = 1186041882
> write(9, "nfsd 129.132.10.33 1186043682 @a"..., 1024) = -1 EINVAL
> (Invalid argument)
> write(9, "\n", 1) = -1 EINVAL (Invalid argument)
> close(9) = 0
> munmap(0xb7f41000, 4096) = 0
> open("/proc/net/rpc/nfsd.export/channel", O_WRONLY|O_CREAT|O_TRUNC,
> 0666) = 9
> fstat64(9, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
> mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
> 0) = 0xb7f41000
> write(9, "@anbuehle,@anhorni,@antoinet,@ap"..., 1024) = -1 EINVAL
> (Invalid argument)
> time(NULL) = 1186041882
> write(9, "/export/groups/grossm/h1/home/gr"..., 68) = -1 ENOENT (No such
> file or directory)
> close(9) = 0
> munmap(0xb7f41000, 4096) = 0
> open("/proc/fs/nfsd/filehandle", O_RDWR) = 9
> fstat64(9, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
> mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
> 0) = 0xb7f41000
> write(9, "@anbuehle,@anhorni,@antoinet,@ap"..., 1066) = -1 EPERM
> (Operation not permitted)
> ...

Yup, the snprintf() in the patch would've truncated the input string.

Steve (D), you should check the return of snprintf() and compare against
the size specified (NFSCLNT_IDMAX+1) and do a graceful cleanup + print
an error message to the user, when detecting truncation of input:


err = snprintf(my_client.m_hostname, (NFSCLNT_IDMAX+1), "%s", *n?n:"DEFAULT");
if (err >= (NFSCLNT_IDMAX+1)) {
printf("too large input string ...\n");
/* cleanups and graceful exit */
}


Sorry, I don't have rpc.mountd sources nearby, so cannot make a patch
myself (I'm an exclusively kernel guy :-)


Thanks,
Satyam

2007-08-03 14:52:04

by Steve Dickson

[permalink] [raw]
Subject: Re: rpc.mountd crashes when extensively using netgroups



Satyam Sharma wrote:
> Hi,
>
>
> On Thu, 2 Aug 2007, Stefan Walter wrote:
>
>> Steve Dickson wrote:
>>> Stefan Walter wrote:
>>>> We do this on a much larger scale though. The bug we ran into is
>>>> in line 96 in utils/mountd/auth.c. The strcpy can corrupt
>>>> memory when it copies the string returned by client_compose() to
>>>> my_client.m_hostname which has a fixed size of 1024 bytes. For our
>>>> example above, client_compose() returns "@joe,@jane"
>>>> for any machine in the offices_1 netgroup. Unfortunately we have
>>>> a machine to which roughly 150 netgroups like @joe or @jane
>>> export to and client_compose() returns a string over 1300 bytes
>>>> long and rpc.mountd nicely segfaults.
>>>>
>>>> To prevent the crash is of course trivial: Inserting a simple
>>>> 'if (strlen(n) > 1024) return NULL;' before line 96 does the job.
>>> Does the attached patch help?
>>>
>> rpc.mountd does not crash anymore but I get a 'permission denied' when
>> trying
>> to mount a share. Doing an 'strace rpc.mountd -F' reveals:
>>
>> ...
>> open("/proc/net/rpc/auth.unix.ip/channel", O_WRONLY|O_CREAT|O_TRUNC,
>> 0666) = 9
>> fstat64(9, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
>> mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
>> 0) = 0xb7f41000
>> time(NULL) = 1186041882
>> write(9, "nfsd 129.132.10.33 1186043682 @a"..., 1024) = -1 EINVAL
>> (Invalid argument)
>> write(9, "\n", 1) = -1 EINVAL (Invalid argument)
>> close(9) = 0
>> munmap(0xb7f41000, 4096) = 0
>> open("/proc/net/rpc/nfsd.export/channel", O_WRONLY|O_CREAT|O_TRUNC,
>> 0666) = 9
>> fstat64(9, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
>> mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
>> 0) = 0xb7f41000
>> write(9, "@anbuehle,@anhorni,@antoinet,@ap"..., 1024) = -1 EINVAL
>> (Invalid argument)
>> time(NULL) = 1186041882
>> write(9, "/export/groups/grossm/h1/home/gr"..., 68) = -1 ENOENT (No such
>> file or directory)
>> close(9) = 0
>> munmap(0xb7f41000, 4096) = 0
>> open("/proc/fs/nfsd/filehandle", O_RDWR) = 9
>> fstat64(9, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
>> mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
>> 0) = 0xb7f41000
>> write(9, "@anbuehle,@anhorni,@antoinet,@ap"..., 1066) = -1 EPERM
>> (Operation not permitted)
>> ...
>
> Yup, the snprintf() in the patch would've truncated the input string.
>
> Steve (D), you should check the return of snprintf() and compare against
> the size specified (NFSCLNT_IDMAX+1) and do a graceful cleanup + print
> an error message to the user, when detecting truncation of input:
>
>
> err = snprintf(my_client.m_hostname, (NFSCLNT_IDMAX+1), "%s", *n?n:"DEFAULT");
> if (err >= (NFSCLNT_IDMAX+1)) {
> printf("too large input string ...\n");
> /* cleanups and graceful exit */
> }
>
cool... thanks!

steved.