On 6/5/24 9:35 AM, Christian Brauner wrote: > On Tue, Jun 04, 2024 at 12:17:39PM -0500, Eric Sandeen wrote: >> Multiple filesystems take uid and gid as options, and the code to >> create the ID from an integer and validate it is standard boilerplate >> that can be moved into common parsing helper functions, so do that for >> consistency and less cut&paste. >> >> This also helps avoid the buggy pattern noted by Seth Jenkins at >> https://lore.kernel.org/lkml/CALxfFW4BXhEwxR0Q5LSkg-8Vb4r2MONKCcUCVioehXQKr35eHg@xxxxxxxxxxxxxx/ >> because uid/gid parsing will fail before any assignment in most >> filesystems. >> >> With this in place, filesystem parsing is simplified, as in >> the patch at >> https://git.kernel.org/pub/scm/linux/kernel/git/sandeen/linux.git/commit/?h=mount-api-uid-helper&id=480d0d3c6699abfbb174b1bf2ab2bbeeec4fe911 >> >> Note that FS_USERNS_MOUNT filesystems still need to do additional >> checking with k[ug]id_has_mapping(), I think. >> >> Thoughts? Is this useful / worthwhile? If so I can send a proper >> 2-patch series ccing the dozen or so filesystems the 2nd patch will >> touch. :) >> >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> >> --- > > Seems worthwhile to me. Ideally you'd funnel through the fc->user_ns smh > so we can do the k[ug]id_has_mapping() checks right in these parsing > helpers. Yep that bothered me too, but we don't have fc here and getting to it looks... tricky. And on a sidebar with Al he seemed to not want that in here, though I'm not quite sure why not. (the reason we don't have fc in the parsers now is because of changes made to support RBD in 7f5d38141e30) Perhaps it's worth getting this far, and fight that battle another day? Thanks, -Eric
