Re: [REPOST PATCH] nfs: fix port value parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 28/6/22 22:34, Trond Myklebust wrote:
On Tue, 2022-06-28 at 08:25 +0800, Ian Kent wrote:
The valid values of nfs options port and mountport are 0 to
USHRT_MAX.

The fs parser will return a fail for port values that are negative
and the sloppy option handling then returns success.

But the sloppy option handling is meant to return success for invalid
options not valid options with invalid values.

Parsing these values as s32 rather than u32 prevents the parser from
returning a parse fail allowing the later USHRT_MAX option check to
correctly return a fail in this case. The result check could be
changed
to use the int_32 union variant as well but leaving it as a uint_32
check avoids using two logical compares instead of one.

Signed-off-by: Ian Kent <raven@xxxxxxxxxx>
---
  fs/nfs/fs_context.c |    4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 9a16897e8dc6..f4da1d2be616 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -156,14 +156,14 @@ static const struct fs_parameter_spec
nfs_fs_parameters[] = {
         fsparam_u32   ("minorversion",  Opt_minorversion),
         fsparam_string("mountaddr",     Opt_mountaddr),
         fsparam_string("mounthost",     Opt_mounthost),
-       fsparam_u32   ("mountport",     Opt_mountport),
+       fsparam_s32   ("mountport",     Opt_mountport),
         fsparam_string("mountproto",    Opt_mountproto),
         fsparam_u32   ("mountvers",     Opt_mountvers),
         fsparam_u32   ("namlen",        Opt_namelen),
         fsparam_u32   ("nconnect",      Opt_nconnect),
         fsparam_u32   ("max_connect",   Opt_max_connect),
         fsparam_string("nfsvers",       Opt_vers),
-       fsparam_u32   ("port",          Opt_port),
+       fsparam_s32   ("port",          Opt_port),
         fsparam_flag_no("posix",        Opt_posix),
         fsparam_string("proto",         Opt_proto),
         fsparam_flag_no("rdirplus",     Opt_rdirplus),


Why don't we just check for the ENOPARAM return value from fs_parse()?

In this case I think the return will be EINVAL.

I think that's a bit to general for this case.

This seemed like the most sensible way to fix it.


Ian




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux