[BusyBox 0000323]: mount.c - passing "-t" option sometimes causes memory copy outside of RAM in the linux kernel

Sat Oct 15 01:04:35 UTC 2005

The following issue has been CLOSED 
====================================================================== 
http://busybox.net/bugs/view.php?id=323 
====================================================================== 
Reported By:                Kasey Erickson
Assigned To:                BusyBox
====================================================================== 
Project:                    BusyBox
Issue ID:                   323
Category:                   Other
Reproducibility:            random
Severity:                   major
Priority:                   normal
Status:                     closed
Resolution:                 no change required
Fixed in Version:           
====================================================================== 
Date Submitted:             07-05-2005 14:40 PDT
Last Modified:              10-14-2005 18:04 PDT
====================================================================== 
Summary:                    mount.c - passing "-t" option sometimes causes
memory copy outside of RAM in the linux kernel
Description: 
uClinux 2.4.22, busybox 1.00-pre3, util-linux/mount.c, mount_main() - When
passing "-t" to mount, optarg's value is copied to filesystemType.  This
becomes a problem when the address copied from optarg to filesystemType is
less than TASK_SIZE (4096 in linux) bytes from the end of RAM. 
linux/fs/namespace.c, copy_mount_options() copies 4096 bytes (on our
platform) for each pointer passed to it.  Roughly 40% of the time the
address optarg holds is 200 bytes from the end of RAM.  This causes the
copy in copy_mount_options to exceed memory.  access_ok() in the kernel
should catch this, but in uClinux it is defined as "0".

By inspection it appears that the same problem can occur to "devices" and
"string_flags" in mount_main.  I haven't seen these pointers produce
problems at run-time though.

A patch that works for me is attached.

====================================================================== 

---------------------------------------------------------------------- 
 landley - 09-01-05 20:15  
---------------------------------------------------------------------- 
A major rewrite of the mount code went in on 8/10.  Does this issue apply
to the new mount code?  Can you still reproduce a problem? 

---------------------------------------------------------------------- 
 Kasey Erickson - 09-02-05 09:27  
---------------------------------------------------------------------- 
After doing a quick code inspection of mount.c (-r11289) the condition
still appears to be present. 

---------------------------------------------------------------------- 
 Kasey Erickson - 10-13-05 10:35  
---------------------------------------------------------------------- 
I see the status of this bug is listed as "feedback".  Do I need to change
it so it will be on the list of bugs to fix?  If so, how do I change its
status?

---------------------------------------------------------------------- 
 landley - 10-13-05 16:52  
---------------------------------------------------------------------- 
First of all, your patch does not apply to the new mount.

Secondly, from your description your patch is a workaround for a kernel
bug.  We're passing a valid pointer to a string to the kernel, and our
string is properly terminated and does not go off the end of memory.  From
your description, your buggy kernel then does something _stupid_ (using a
memcpy with a fixed length rather than a strncpy).  Your workaround merely
perturbs memory layout so the bug coincidentally doesn't happen to you
anymore.

What exactly is wrong with busybox's behavior?  Why is fixing your kernel
instead not the correct course of action here, since your kernel is what's
broken?  I don't understand quite what you want, here... 

---------------------------------------------------------------------- 
 Kasey Erickson - 10-14-05 16:28  
---------------------------------------------------------------------- 
Withdrawn, not a bug.  This issue has been discussed on the uclinux forum
before:
http://mailman.uclinux.org/pipermail/uclinux-dev/2002-January/006405.html,
http://mailman.uclinux.org/pipermail/uclinux-dev/2001-November/005440.html,
http://mailman.uclinux.org/pipermail/uclinux-dev/2001-November/005439.html,
http://mailman.uclinux.org/pipermail/uclinux-dev/2002-January/006406.html.

Turns out the kernel we're using (a derivation of LxNETES which came from
some version of uclinux) didn't include a change made over 3 years ago
that takes care of the condition (in fs/namespace.c). 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
07-05-05 14:40  Kasey Erickson New Issue                                    
07-05-05 14:40  Kasey Erickson File Added: mount.c.patch                    
09-01-05 20:15  landley        Note Added: 0000486                          
09-01-05 20:15  landley        Status                   assigned => feedback
09-02-05 09:27  Kasey Erickson Note Added: 0000487                          
10-13-05 10:34  Kasey Erickson Issue Monitored: landley                     
10-13-05 10:35  Kasey Erickson Note Added: 0000620                          
10-13-05 16:52  landley        Note Added: 0000621                          
10-14-05 16:28  Kasey Erickson Note Added: 0000622                          
10-14-05 18:04  landley        Status                   feedback => closed  
10-14-05 18:04  landley        Resolution               open => no change
required
======================================================================