Thread (69 messages) 69 messages, 13 authors, 2023-10-04

Re: [RFC PATCH 2/3] add statmnt(2) syscall

From: Miklos Szeredi <miklos@szeredi.hu>
Date: 2023-09-20 09:42:30
Also in: linux-fsdevel, linux-man, linux-security-module, lkml

On Tue, 19 Sept 2023 at 23:28, Matthew House [off-list ref] wrote:
More generally speaking, the biggest reason I dislike the current single-
buffer interface is that the output is "all or nothing": either the caller
has enough space in the buffer to store every single string, or it's unable
to get any fields at all, just an -EOVERFLOW. There's no room for the
caller to say that it just wants the integer fields and doesn't care about
the strings. Thus, to reliably call statmnt() on an arbitrary mount, the
ability to dynamically allocate memory is effectively mandatory. The only
real solution to this would be additional statx-like flags to select the
returned strings.
It's already there:

#define STMT_MNT_ROOT 0x00000008U /* Want/got mnt_root  */
#define STMT_MNT_POINT 0x00000010U /* Want/got mnt_point */
#define STMT_FS_TYPE 0x00000020U /* Want/got fs_type */

For example, it's perfectly fine to do the following, and it's
guaranteed not to return EOVERFLOW:

        struct statmnt st;
        unsigned int mask = STMT_SB_BASIC | STMT_MNT_BASIC;

        ret = statmount(mnt_id, mask, &st, sizeof(st), flags);
Besides that, if the caller is written in standard C but doesn't want to
use malloc(3) to allocate the buffer, then its helper function must be
written very carefully (with a wrapper struct around the header and data)
to satisfy the aliasing rules, which forbid programs from using a struct
statmnt * pointer to read from a declared char[N] array.
I think you interpret aliasing rules incorrectly.  The issue with
aliasing is if you access the same piece of memory though different
types.  Which is not the case here.  In fact with the latest
incarnation of the interface[1] there's no need to access the
underlying buffer at all:

        printf("mnt_root: <%s>\n", st->str + st->mnt_root);

So the following is perfectly safe to do (as long as you don't care
about buffer overflow):

        char buf[10000];
        struct statmnt *st = (void *) buf;

        ret = statmount(mnt_id, mask, st, sizeof(buf), flags);

If you do care about handling buffer overflows, then dynamic
allocation is the only sane way.

And before you dive into how this is going to be horrible because the
buffer size needs to be doubled an unknown number of times, think a
bit:  have you *ever* seen a line in /proc/self/mountinfo longer than
say 1000 characters?   So if the buffer starts out at 64k, how often
will this doubling happen?   Right: practically never.  Adding
complexity to handle this case is nonsense, as I've said many times.
And there is definitely nonzero complexity involved (just see the
special casing in getxattr and listxattr implementations all over the
place).

Thanks,
Miklos

[1] git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git#statmount-v2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help