Bug 138994 - autofs drops mounts due to buffer overflow when using program maps
autofs drops mounts due to buffer overflow when using program maps
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 3
Classification: Red Hat
Component: autofs (Show other bugs)
3.0
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jeffrey Moyer
:
Depends On:
Blocks: 132991
  Show dependency treegraph
 
Reported: 2004-11-12 08:22 EST by Neil Horman
Modified: 2007-11-30 17:07 EST (History)
4 users (show)

See Also:
Fixed In Version: RHBA-2005-178
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-05-19 18:05:44 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
patch to allow the stdin receive buffer mapent grow to an arbitrary length based on the amount of data read from stdin (2.24 KB, patch)
2004-11-12 08:26 EST, Neil Horman
no flags Details | Diff
Modified version of previous patch, allows for large program maps. (3.37 KB, patch)
2004-11-19 16:41 EST, Jeffrey Moyer
no flags Details | Diff
Fix previous broken patch. (3.41 KB, patch)
2004-12-20 10:31 EST, Jeffrey Moyer
no flags Details | Diff

  None (edit)
Description Neil Horman 2004-11-12 08:22:29 EST
Description of problem:
when using program type maps, autofs/automount can drop nfs mounts
when the amount of data returned by stdin from running the program
exceedes the size of the buffer the data is being read into (currently
set at 1 page).

Version-Release number of selected component (if applicable):
4.1.3

How reproducible:
always

Steps to Reproduce:

1.set up an nfs server, and export 500 mounts on it.  Ensure that the
string which represents the path of the mount is at least 10
characters long.  This will ensure that when auto.net (the default
script for finding program type autofs mounts will generate more than
4096 bytes on stdout for automount to read on stdin.

2.configure autofs to have a program mount on /net running auto.net to
resolve mounts

3.start the autofs service

4.cd /net/<nfs server name>

5.execute ls
  
Actual results:
some of the mounts exported by the server will not be mounted.  The
number will vary based on the string length of the mount paths.

Expected results:
all exported mounts from the nfs server should be mounted

Additional info:
I've tested this with the latest code from CVS and the problem occurs
there as well.
Comment 1 Neil Horman 2004-11-12 08:26:37 EST
Created attachment 106563 [details]
patch to allow the stdin receive buffer mapent grow to an arbitrary length based on the amount of  data read from stdin

The problem here appears to be that the buffer which receives the data from
stdin in automount (which is generated from the auto.net script) is a fixed
size of 4096 bytes.  If auto.net reports the exports from an nfs server, and
those strings total more than 4096 bytes, and remaining data is discarded,
leading to lost mounts.  This patch detects when mapent is full, and grows its
size by 1 page each time the full condition is detected, allowing mapent to
grow to whatever size is needed for the reporting server.

This may actually be a problem in the other map type modules as well, but since
I haven't had a chance to test with any of them, I've only patched the
lookup_program module.
Comment 2 Neil Horman 2004-11-12 13:19:23 EST
Customer feedback reports that this patch successfully fixes this issue.
Comment 3 Jeffrey Moyer 2004-11-15 20:11:42 EST
Thanks for the patch, Neil.  I've got an updated version which fixes a
potential buffer overrun and some memory leaks which I'll post after
testing.

To answer your question, this problem does not exist in the other
lookup modules because they are implemented a bit differently.  For
each of the other modules, the mapent array is used to hold only a
single entry (as its name suggests), and once that entry is complete,
it is updated in the internal ghosting cache.  Program maps do not
support ghosting, and thus their entries are not cached.  As such, we
read the entire map into the mapent array.
Comment 4 Neil Horman 2004-11-15 21:30:21 EST
Thanks Jeff, both for the patch review and the explination on the
other modules. Shall I post this patch to the autofs mailing list, or
will you just take care of it as part of your changeset with the extra
leak fixes?
Comment 5 Jeffrey Moyer 2004-11-15 23:12:18 EST
No worries.  I will take care of submitting upstream.  Worry not, I
will mention the patch came from you, since you did all of the hard work!

Thanks again!
Comment 6 Jeffrey Moyer 2004-11-19 16:42:00 EST
Created attachment 107092 [details]
Modified version of previous patch, allows for large program maps.

Sorry for the delay in posting the patch.  This version has been tested and
submitted upstream.
Comment 9 James Antill 2004-12-17 11:50:58 EST
 The patch for is wrong in the fact that the code changed from...

                                if (!quoted && ch == '\n') {
                                        *mapp = '\0';
                                        state = st_done;
                                } else if (mapp - mapent < MAPENT_MAX_LEN - 1) {
[...]
                                }
                                break;

...to...

                                if (!quoted && ch == '\n') {
                                        *mapp = '\0';
                                        state = st_done;
                                /* We overwrite up to 3 characters, so we
                                 * need to make sure we have enough room
                                 * in the buffer for this. */
                                } else if (mapp - mapent >
                                           ((MAPENT_MAX_LEN+1) * alloci) - 3) {
[...]
                                }
                                /*
                                 * Eat \ quoting \n, otherwise pass it
                                 * through for the parser
                                 */
                                if (quoted) {
                                        if (ch == '\n')
                                                *mapp++ = ' ';
                                        else {
                                                *mapp++ = '\\';
                                                *mapp++ = ch;
                                        }
                                } else
                                        *mapp++ = ch;

...this new end bit was previously inside the second if ... but is now outside
and so also runs in the (!quoted && ch == '\n') case.
Comment 10 Jeffrey Moyer 2004-12-17 12:23:52 EST
Good catch, James, thank you.  I'll put together another patch when I
get the chance.
Comment 11 Jeffrey Moyer 2004-12-20 10:31:25 EST
Created attachment 108901 [details]
Fix previous broken patch.

This patch is currently untested, but looks to preserve the scope of the if in
question.
Comment 17 Dennis Gregorovic 2005-05-19 18:05:44 EDT
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2005-177.html

Note You need to log in before you can comment on or make changes to this bug.