linux-kernel - Re: [PATCH] vfs: Don't reject unknown parameters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJfpegv_zY6w6=pOL0x=sjuQmGae0ymOafZXjyAdNEHj+EKyNA@mail.gmail.com>
Date:   Fri, 13 Dec 2019 10:15:03 +0100
From:   Miklos Szeredi <miklos@...redi.hu>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Laura Abbott <labbott@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Ilya Dryomov <idryomov@...il.com>,
        David Howells <dhowells@...hat.com>,
        Jeremi Piotrowski <jeremi.piotrowski@...il.com>,
        Linux FS Devel <linux-fsdevel@...r.kernel.org>,
        Phillip Lougher <phillip@...ashfs.org.uk>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] vfs: Don't reject unknown parameters

On Thu, Dec 12, 2019 at 10:36 PM Al Viro <viro@...iv.linux.org.uk> wrote:

> So you could bloody well just leave recognition (and handling) of "source"
> to the caller, leaving you with just this:
>
>         if (strcmp(param->key, "source") == 0)
>                 return -ENOPARAM;
>         /* Just log an error for backwards compatibility */
>         errorf(fc, "%s: Unknown parameter '%s'", fc->fs_type->name, param->key);
>         return 0;

Which is fine for the old mount(2) interface.

But we have a brand new API as well; do we really need to carry these
backward compatibility issues forward?  I mean checking if a
param/flag is supported or not *is* useful and lacking that check is
the source of numerous headaches in legacy interfaces (just take the
open(2) example and the introduction of O_TMPFILE).

Just need a flag in fc indicating if this option comes from the old interface:

         if (strcmp(param->key, "source") == 0)
                 return -ENOPARAM;
         /* Just log an error for backwards compatibility */
         errorf(fc, "%s: Unknown parameter '%s'", fc->fs_type->name,
param->key);
         return fc->legacy ? 0 : -ENOPARAM;

And TBH, I think that logic applies to the common flags as well.  Some
of these simply make no sense on the new interface ("silent",
"posixacl") and some are ignored for lots of filesystems ("sync",
"dirsync", "mand", "lazytime").  It would also be nice to reject "rw"
for read-only filesystems.

I have sent patches for the above numerous times, all been ignored by
DavidH and Al.  While this seems minor now, I think getting this
interface into a better shape as early as possible may save lots more
headaches later...

Thanks,
Miklos