[lkml]   [2020]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 07/17] fsinfo: Add fsinfo() syscall to query filesystem information [ver #17]
    Aleksa Sarai <> wrote:

    > > If params is given, all of params->__reserved[] must be 0.
    > I would suggest that rather than having a reserved field for future
    > extensions, you make use of copy_struct_from_user() and have extensible
    > structs:

    Yeah. I seem to recall that special support was required for 6-arg syscalls
    on some arches, though I could move the dfd argument into the parameter block
    and make AT_FDCWD the default.

    > I dropped the "const" on fsinfo_params because the planned CHECK_FiELDS
    > feature for extensible-struct syscalls requires writing to the struct.

    Ummm... Why? You shouldn't be trying to alter the parameters structure. It
    could feasibly be stored static const in userspace (though I'm not sure how
    likely it would be that someone would do that).

    > I also switched the flags field to u64 because CHECK_FiELDS is intended to
    > use (1<<63) for all syscalls (this has the nice benefit of removing the need
    > of a padding field entirely).

    struct fsinfo_params {
    __u32 flags;
    __u32 at_flags;
    __u32 request;
    __u32 Nth;
    __u32 Mth;

    What padding? ;-)

    Though possibly the struct does need forcing to 64-bit alignment for future

    > > dfd, filename and params->at_flags indicate the file to query. There is no
    > > equivalent of lstat() as that can be emulated with fsinfo() by setting
    > > AT_SYMLINK_NOFOLLOW in params->at_flags.
    > Minor gripe -- can we make the default be AT_SYMLINK_NOFOLLOW and you
    > need to explicitly pass AT_SYMLINK_FOLLOW? Accidentally following
    > symlinks is a constant source of security bugs.

    Someone else has said that all new syscalls should be using RESOLVE_* flags in
    preference to AT_* flags (even though RESOLVE_* flags are not a superset of
    AT_* flags and appear to be in a header named specifically for the openat2()
    syscall, not generic).

    I'm not sure who authored openat2.h, but they went with a RESOLVE_NO_SYMLINKS
    rather than a RESOLVE_SYMLINKS ;-)

    > > There is also no equivalent of fstat() as that can be emulated by
    > > passing a NULL filename to fsinfo() with the fd of interest in dfd.
    > Presumably you also need to pass AT_EMPTY_PATH?

    Actually, you need to set FSINFO_FLAGS_QUERY_FD in fsinfo_params::flags. I
    need to update the description for this.

    > Sounds good, though I think we should zero-fill the tail end of the
    > buffer (if the buffer is larger than the in-kernel one).

    I do that. I should make it clearer in the patch description.


     \ /
      Last update: 2020-02-28 15:45    [W:11.370 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site