Red Hat Bugzilla – Bug 138994
autofs drops mounts due to buffer overflow when using program maps
Last modified: 2007-11-30 17:07:05 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.
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.
Customer feedback reports that this patch successfully fixes this issue.
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.
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?
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!
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.
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.
Good catch, James, thank you. I'll put together another patch when I get the chance.
Created attachment 108901 [details] Fix previous broken patch. This patch is currently untested, but looks to preserve the scope of the if in question.
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