On Apr 20, 2008, at 6:34 PM, Steinar H. Gunderson wrote:
> Hi guys,
>
> Would you please consider this patch for inclusion into upstream? It
> looks
> more than reasonable to me, at least.
What if /etc/mtab is a symlink to a valid writable file that is not /
proc/mounts? The test you introduce below will prevent that case from
working properly.
Is there a security issue with creating a file in / when /proc isn't
mounted, or is this just an inconvenience?
> /* Steinar */
>
> On Thu, Apr 17, 2008 at 04:09:28PM -0400, Joey Hess wrote:
>> Hmm, nmudiff did not send in the patch I expected. Here is a
>> corrected/tested one.
>>
>> --
>> see shy jo
>
>> diff -ur foo/nfs-utils-1.1.2/debian/changelog nfs-utils-1.1.2/
>> debian/changelog
>> --- foo/nfs-utils-1.1.2/debian/changelog 2008-04-17
>> 16:06:27.000000000 -0400
>> +++ nfs-utils-1.1.2/debian/changelog 2008-04-17 15:33:41.000000000
>> -0400
>> @@ -1,3 +1,10 @@
>> +nfs-utils (1:1.1.2-2.1) UNRELEASED; urgency=low
>> +
>> + * Avoid writing to or fchmodding /etc/mtab if it is a symlink.
>> + Closes: #476577
>> +
>> + -- Joey Hess <[email protected]> Thu, 17 Apr 2008 15:30:49 -0400
>> +
>> nfs-utils (1:1.1.2-2) unstable; urgency=low
>>
>> * Remove ${misc:Depends} from binary variables -- it is not used,
>> and not
>> diff -ur foo/nfs-utils-1.1.2/utils/mount/fstab.c nfs-utils-1.1.2/
>> utils/mount/fstab.c
>> --- foo/nfs-utils-1.1.2/utils/mount/fstab.c 2008-03-14
>> 11:46:29.000000000 -0400
>> +++ nfs-utils-1.1.2/utils/mount/fstab.c 2008-04-17
>> 15:40:01.000000000 -0400
>> @@ -52,7 +52,7 @@
>> return var_mtab_does_not_exist;
>> }
>>
>> -static int
>> +int
>> mtab_is_a_symlink(void) {
>> get_mtab_info();
>> return var_mtab_is_a_symlink;
>> diff -ur foo/nfs-utils-1.1.2/utils/mount/fstab.h nfs-utils-1.1.2/
>> utils/mount/fstab.h
>> --- foo/nfs-utils-1.1.2/utils/mount/fstab.h 2008-03-14
>> 11:46:29.000000000 -0400
>> +++ nfs-utils-1.1.2/utils/mount/fstab.h 2008-04-17
>> 15:38:30.000000000 -0400
>> @@ -7,6 +7,7 @@
>> #define _PATH_FSTAB "/etc/fstab"
>> #endif
>>
>> +int mtab_is_a_symlink(void);
>> int mtab_is_writable(void);
>> int mtab_does_not_exist(void);
>>
>> diff -ur foo/nfs-utils-1.1.2/utils/mount/mount.c nfs-utils-1.1.2/
>> utils/mount/mount.c
>> --- foo/nfs-utils-1.1.2/utils/mount/mount.c 2008-03-14
>> 11:46:29.000000000 -0400
>> +++ nfs-utils-1.1.2/utils/mount/mount.c 2008-04-17
>> 15:39:34.000000000 -0400
>> @@ -257,6 +257,13 @@
>> return EX_SUCCESS;
>> }
>>
>> + /* Avoid writing if the mtab is a symlink to /proc/mounts,
>> since
>> + that would create a file /proc/mounts in case the proc
>> filesystem
>> + is not mounted, and the fchmod below would also fail. */
>> + if (mtab_is_a_symlink()) {
>> + return EX_SUCCESS;
>> + }
>> +
>> lock_mtab();
>>
>> mtab = nfs_setmntent(MOUNTED, "a+");
>
>
>
>
> --
> Homepage: http://www.sesse.net/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
Chuck Lever wrote:
> What if /etc/mtab is a symlink to a valid writable file that is not /
> proc/mounts? The test you introduce below will prevent that case from
> working properly.
Note that util-linux mount does the same thing in this case, so this must
be an uncommon case.
umount.nfs also avoids writing to /etc/mtab if it is a symlink..
> Is there a security issue with creating a file in / when /proc isn't
> mounted, or is this just an inconvenience?
The worse problem is that mount.nfs fails with a nonzero exit code if
/proc/ is mounted, since the fchmod of /proc/mounts fails.
--
see shy jo
Hi Joey-
On Apr 21, 2008, at 12:53 PM, Joey Hess wrote:
> Chuck Lever wrote:
>> What if /etc/mtab is a symlink to a valid writable file that is not /
>> proc/mounts? The test you introduce below will prevent that case
>> from
>> working properly.
>
> Note that util-linux mount does the same thing in this case, so this
> must be an uncommon case.
>
> umount.nfs also avoids writing to /etc/mtab if it is a symlink..
In that case, I think the patch description should start out with
"Make mount.nfs handle symlinked /etc/mtab the way umount.nfs and util-
linux handle it." Then the description should explain why this
important.
That provides much better documentation of the change and why it is
needed. Having the util-linux precedent is especially good to know,
and demonstrates that the right homework was done.
>> Is there a security issue with creating a file in / when /proc isn't
>> mounted, or is this just an inconvenience?
>
> The worse problem is that mount.nfs fails with a nonzero exit code if
> /proc/ is mounted, since the fchmod of /proc/mounts fails.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com