mountall ignores NFS mount option "timeo"

[Rolf Anders]

If the mount option "timeo" is specified for an NFS mount in /etc/fstab,
mountall does not pass it to mount.nfs. The bug is in the function
cut_options() in mountall.c. It is called with the following arguments:

        opts = cut_options (NULL, mnt, "showthrough", "optional",
                            "bootwait", "nobootwait", "timeout",
                            NULL);

Although it should only remove the specified options, it also removes
any option that is a prefix of one of those, e.g. "timeo".

The call to strncmp() in line 661 compares only up to the length of the
option from the fstab, without comparing the length of both strings.
Hence I propose the following patch:

*(updated patch in comment #6).
--- mountall-2.36/src/mountall.c.orig 2012-08-24 20:25:07.445892095 +0200
+++ mountall-2.36/src/mountall.c 2012-08-24 21:29:10.098595843 +0200
@@ -658,7 +658,8 @@

                va_copy (options, args);
                while ((option = va_arg (options, const char *)) != NULL)
{
- if (j && ! strncmp (opts + i, option, j))
+ if (j && option[j] == '\0'
+ && ! strncmp (opts + i, option, j))
                                break;
                }
                va_end (options);

[Impact]
Mounts that have options that are substrings of (showthrough, optional, bootwait, nobootwait or timeout) Are being stripped by mountall and not being passed to the mount. Such is the case of timeo in nfs.

The change is minimal, and simply checks that the option being stripped matches the length of option being checked against. This solves the case where one is a substring of the other.

[Test Case]
1. Create an nfs mount option in /etc/fstab with the timeo=2 option. Like the below.
server:/raid /raid nfs user,timeo=2,timeout=6 0 0
2. Run mountall as root
3. run mount and verify that the nfs mount specified above has the options user,timeo=2 , but not timeout=6.

[Regression Potential]
Regression potential is minimal, but regressions are most likely to manifest themselves in mount options disappearing or new options appearing (some new options appearing will be valid as is the case of timeo).

[Dave Chiluk (chiluk)]

I've tested and verified the above fix with the above linked branches. Please consider this for raring and an sru for precise and quantal.

[Steve Langasek (vorlon)]

Hi Dave,

There's an issue with your proposed patch; if /etc/fstab contains very long option names, option[j] == '\0' will look past the end of the string, possibly leading to a segfault.

Could you adjust your patch to use strlen(option) instead?

[Dave Chiluk (chiluk)]

Sounds good, I was holding off on filling out the SRU until I had more fully vetted the patch. Thanks for taking a look at it.

[Dave Chiluk (chiluk)]

[Impact]
 * Mounts that have options that are substrings of (showthrough, optional, bootwait, nobootwait or timeout) Are
    being stripped by mountall and not being passed to the mount. Such is the case of timeo in nfs.

    The change is minimal, and simply checks that the option being stripped matches the length of option being checked against.
    This solves the case where one is a substring of the other.

[Test Case]
1. Create an nfs mount option in /etc/fstab with the timeo=2 option. Like the below.
server:/raid /raid nfs user,timeo=2,timeout=6 0 0
2. Run mountall as root
3. run mount and verify that the nfs mount specified above has the options user,timeo=2 , but not timeout=6.

[Regression Potential]
Regression potential is minimal, but regressions are most likely to manifest themselves in mount options disappearing or new options appearing (some new options appearing will be valid as is the case of timeo).

[Dave Chiluk (chiluk)]

Here is the updated diff for convenience.

=== modified file 'src/mountall.c'
--- src/mountall.c 2012-10-09 19:32:55 +0000
+++ src/mountall.c 2012-12-19 16:28:45 +0000
@@ -684,7 +684,8 @@

   va_copy (options, args);
   while ((option = va_arg (options, const char *)) != NULL) {
- if (j && ! strncmp (opts + i, option, j))
+ if (j && strlen(option) == j
+ && ! strncmp (opts + i, option, j))
     break;
   }
   va_end (options);

[Steve Langasek (vorlon)]

Patch looks good, thanks. I've pushed it to the bzr branch for raring/unstable; haven't uploaded yet, will probably batch up a few other changes before uploading, but this should be enough to go on with for the SRU.

[Dave Chiluk (chiluk)]

Precise debdiffPrec

[Dave Chiluk (chiluk)]

Quantal Debdiff

[Adam Conrad (adconrad)]

Hello Rolf, or anyone else affected,

Accepted mountall into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/mountall/2.36.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Adam Conrad (adconrad)]

Hello Rolf, or anyone else affected,

Accepted mountall into quantal-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/mountall/2.42ubuntu0.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Rolf Anders (rolf-anders)]

mountall 2.36.3 solves the problem (tested on Ubuntu 12.04). Thanks!

[Launchpad Janitor (janitor)]

This bug was fixed in the package mountall - 2.36.3

---------------
mountall (2.36.3) precise-proposed; urgency=low

  * Adjust parsing of options so mountall doesn't strip options that are
    substrings of these strings (showthrough, optional, bootwait, nobootwait
    or timeout). This fixes the issue where timeo was getting stripped from
    nfs mounts. LP: #1041377.
 -- Dave Chiluk <email address hidden> Wed, 19 Dec 2012 10:21:43 -0600

[Dave Chiluk (chiluk)]

I verified with mountall_2.42ubuntu0.3_amd64.deb for quantal.

[Brian Murray (brian-murray)]

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package mountall - 2.42ubuntu0.3

---------------
mountall (2.42ubuntu0.3) quantal-proposed; urgency=low

  * Adjust parsing of options so mountall doesn't strip options that are
    substrings of these strings (showthrough, optional, bootwait, nobootwait
    or timeout). This fixes the issue where timeo was getting stripped from
    nfs mounts. LP: #1041377.
 -- Dave Chiluk <email address hidden> Wed, 19 Dec 2012 10:21:43 -0600

[Launchpad Janitor (janitor)]

This bug was fixed in the package mountall - 2.47

---------------
mountall (2.47) unstable; urgency=low

  [ Dave Chiluk ]
  * Adjust parsing of options so mountall doesn't strip options that are
    substrings of these strings (showthrough, optional, bootwait, nobootwait
    or timeout). This fixes the issue where timeo was getting stripped from
    nfs mounts. LP: #1041377.

  [ Steve Langasek ]
  * Ensure callbacks are called directly when running with --no-events,
    otherwise the "event" handling of the non-events never finishes and
    mountall hangs. LP: #1099349.

 -- Steve Langasek <email address hidden> Mon, 14 Jan 2013 16:37:36 -0800