On Mon, 7 Oct 2019, Laura Abbott wrote: > On 9/30/19 12:07 PM, Laura Abbott wrote: > > Hi, > > > > Fedora got a bug report https://bugzilla.redhat.com/show_bug.cgi?id=1757104 > > of a failure to parse options with the context mount option. From the > > reporter: > > > > > > $ unshare -rm mount -t tmpfs tmpfs /tmp -o > > 'context="system_u:object_r:container_file_t:s0:c475,c690"' > > mount: /tmp: wrong fs type, bad option, bad superblock on tmpfs, missing > > codepage or helper program, or other error. > > > > > > Sep 30 16:50:42 kernel: tmpfs: Unknown parameter 'c690"' > > > > I haven't asked the reporter to bisect yet but I'm suspecting one of the > > conversion to the new mount API: > > > > $ git log --oneline v5.3..origin/master mm/shmem.c > > edf445ad7c8d Merge branch 'hugepage-fallbacks' (hugepatch patches from > > David Rientjes) > > 19deb7695e07 Revert "Revert "Revert "mm, thp: consolidate THP gfp handling > > into alloc_hugepage_direct_gfpmask"" > > 28eb3c808719 shmem: fix obsolete comment in shmem_getpage_gfp() > > 4101196b19d7 mm: page cache: store only head pages in i_pages > > d8c6546b1aea mm: introduce compound_nr() > > f32356261d44 vfs: Convert ramfs, shmem, tmpfs, devtmpfs, rootfs to use the > > new mount API > > 626c3920aeb4 shmem_parse_one(): switch to use of fs_parse() > > e04dc423ae2c shmem_parse_options(): take handling a single option into a > > helper > > f6490b7fbb82 shmem_parse_options(): don't bother with mpol in separate > > variable > > 0b5071dd323d shmem_parse_options(): use a separate structure to keep the > > results > > 7e30d2a5eb0b make shmem_fill_super() static > > > > > > I didn't find another report or a fix yet. Is it worth asking the reporter > > to bisect? > > > > Thanks, > > Laura > > Ping again, I never heard anything back and I didn't see anything come in > with -rc2 Sorry for not responding sooner, Laura, I was travelling: and dearly hoping that David or Al would take it. I'm afraid this is rather beyond my capability (can I admit that it's the first time I even heard of the "context" mount option? and grepping for "context" has not yet shown me at what level it is handled; and I've no idea of what a valid "context" is for my own tmpfs mounts, to start playing around with its parsing). Yes, I think we can assume that this bug comes from f32356261d44 ("vfs: Convert ramfs, shmem, tmpfs, devtmpfs, rootfs to use the new mount API") or one of shmem_parse ones associated with it; but I'm pretty sure that it's not worth troubling the reporter to bisect. I expect David and Al are familiar with "context", and can go straight to where it's handled, and see what's up. (tmpfs, very tiresomely, supports a NUMA "mpol" mount option which can have commas in it e.g "mpol=bind:0,2": which makes all its comma parsing awkward. I assume that where the new mount API commits bend over to accommodate that peculiarity, they end up mishandling the comma in the context string above.) And since we're on the subject of new mount API breakage in tmpfs, I'll take the liberty of repeating this different case, reported earlier and still broken in rc2: again something that I'd be hard-pressed to fix myself, without endangering some other filesystem's mount parsing:- My /etc/fstab has a line in for one of my test mounts: tmpfs /tlo tmpfs size=4G 0 0 and that "size=4G" is what causes the problem: because each time shmem_parse_options(fc, data) is called for a remount, data (that is, options) points to a string starting with "size=4G,", followed by what's actually been asked for in the remount options. So if I try mount -o remount,size=0 /tlo that succeeds, setting the filesystem size to 0 meaning unlimited. So if then as a test I try mount -o remount,size=1M /tlo that correctly fails with "Cannot retroactively limit size". But then when I try mount -o remount,nr_inodes=0 /tlo I again get "Cannot retroactively limit size", when it should have succeeded (again, 0 here meaning unlimited). That's because the options in shmem_parse_options() are "size=4G,nr_inodes=0", which indeed looks like an attempt to retroactively limit size; but the user never asked "size=4G" there. Hugh
