| Message ID | b6f508ca-b1b2-5f40-7998-e4cff1cf7212@kernel.dk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL] io_uring fixes for 6.0-rc1 | expand |
On Fri, Aug 12, 2022 at 5:46 AM Jens Axboe <axboe@kernel.dk> wrote: > > - Small series improving type safety of the sqe fields (Stefan) This doesn't work AT ALL. A basic allmodconfig build fails with tons of errors. It starts with In function ‘io_kiocb_cmd_sz_check’, inlined from ‘io_prep_rw’ at io_uring/rw.c:38:21: ././include/linux/compiler_types.h:354:45: error: call to ‘__compiletime_assert_802’ declared with attribute error: BUILD_BUG_ON failed: cmd_sz > sizeof(struct io_cmd_data) 354 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^ ././include/linux/compiler_types.h:335:25: note: in definition of macro ‘__compiletime_assert’ 335 | prefix ## suffix(); \ | ^~~~~~ ././include/linux/compiler_types.h:354:9: note: in expansion of macro ‘_compiletime_assert’ 354 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^~~~~~~~~~~~~~~~~~~ ./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’ 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) | ^~~~~~~~~~~~~~~~~~ ./include/linux/build_bug.h:50:9: note: in expansion of macro ‘BUILD_BUG_ON_MSG’ 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) | ^~~~~~~~~~~~~~~~ ./include/linux/io_uring_types.h:496:9: note: in expansion of macro ‘BUILD_BUG_ON’ 496 | BUILD_BUG_ON(cmd_sz > sizeof(struct io_cmd_data)); | ^~~~~~~~~~~~ and goes downhill from there. I don't think this can have seen any testing at all. Linus
On Aug 12, 2022, at 2:28 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Fri, Aug 12, 2022 at 5:46 AM Jens Axboe <axboe@kernel.dk> wrote: >> >> - Small series improving type safety of the sqe fields (Stefan) > > This doesn't work AT ALL. > > A basic allmodconfig build fails with tons of errors. It starts with > > In function ‘io_kiocb_cmd_sz_check’, > inlined from ‘io_prep_rw’ at io_uring/rw.c:38:21: > ././include/linux/compiler_types.h:354:45: error: call to > ‘__compiletime_assert_802’ declared with attribute error: BUILD_BUG_ON > failed: cmd_sz > sizeof(struct io_cmd_data) > 354 | _compiletime_assert(condition, msg, > __compiletime_assert_, __COUNTER__) > | ^ > ././include/linux/compiler_types.h:335:25: note: in definition of > macro ‘__compiletime_assert’ > 335 | prefix ## suffix(); > \ > | ^~~~~~ > ././include/linux/compiler_types.h:354:9: note: in expansion of > macro ‘_compiletime_assert’ > 354 | _compiletime_assert(condition, msg, > __compiletime_assert_, __COUNTER__) > | ^~~~~~~~~~~~~~~~~~~ > ./include/linux/build_bug.h:39:37: note: in expansion of macro > ‘compiletime_assert’ > 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) > | ^~~~~~~~~~~~~~~~~~ > ./include/linux/build_bug.h:50:9: note: in expansion of macro > ‘BUILD_BUG_ON_MSG’ > 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " > #condition) > | ^~~~~~~~~~~~~~~~ > ./include/linux/io_uring_types.h:496:9: note: in expansion of macro > ‘BUILD_BUG_ON’ > 496 | BUILD_BUG_ON(cmd_sz > sizeof(struct io_cmd_data)); > | ^~~~~~~~~~~~ > > and goes downhill from there. > > I don't think this can have seen any testing at all. Wtf? I always run allmodconfig before sending and it also ran testing. I’ll check shortly. Sorry about that, whatever went wrong here.
On 8/12/22 2:44 PM, Jens Axboe wrote: > On Aug 12, 2022, at 2:28 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: >> >> ?On Fri, Aug 12, 2022 at 5:46 AM Jens Axboe <axboe@kernel.dk> wrote: >>> >>> - Small series improving type safety of the sqe fields (Stefan) >> >> This doesn't work AT ALL. >> >> A basic allmodconfig build fails with tons of errors. It starts with >> >> In function ?io_kiocb_cmd_sz_check?, >> inlined from ?io_prep_rw? at io_uring/rw.c:38:21: >> ././include/linux/compiler_types.h:354:45: error: call to >> ?__compiletime_assert_802? declared with attribute error: BUILD_BUG_ON >> failed: cmd_sz > sizeof(struct io_cmd_data) >> 354 | _compiletime_assert(condition, msg, >> __compiletime_assert_, __COUNTER__) >> | ^ >> ././include/linux/compiler_types.h:335:25: note: in definition of >> macro ?__compiletime_assert? >> 335 | prefix ## suffix(); >> \ >> | ^~~~~~ >> ././include/linux/compiler_types.h:354:9: note: in expansion of >> macro ?_compiletime_assert? >> 354 | _compiletime_assert(condition, msg, >> __compiletime_assert_, __COUNTER__) >> | ^~~~~~~~~~~~~~~~~~~ >> ./include/linux/build_bug.h:39:37: note: in expansion of macro >> ?compiletime_assert? >> 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) >> | ^~~~~~~~~~~~~~~~~~ >> ./include/linux/build_bug.h:50:9: note: in expansion of macro >> ?BUILD_BUG_ON_MSG? >> 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " >> #condition) >> | ^~~~~~~~~~~~~~~~ >> ./include/linux/io_uring_types.h:496:9: note: in expansion of macro >> ?BUILD_BUG_ON? >> 496 | BUILD_BUG_ON(cmd_sz > sizeof(struct io_cmd_data)); >> | ^~~~~~~~~~~~ >> >> and goes downhill from there. >> >> I don't think this can have seen any testing at all. > > Wtf? I always run allmodconfig before sending and it also ran testing. > I?ll check shortly. Sorry about that, whatever went wrong here. My test box is still on the same sha from this morning, which is: commit 2ae08b36c06ea8df73a79f6b80ff7964e006e9e3 (origin/master, origin/HEAD) Merge: 21f9c8a13bb2 8bb5e7f4dcd9 Author: Linus Torvalds <torvalds@linux-foundation.org> Date: Thu Aug 11 09:23:08 2022 -0700 Merge tag 'input-for-v5.20-rc0' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input with io_uring-6.0 (ff34d8d06a1f16b6a58fb41bfbaa475cc6c02497) and block-6.0 (aa0c680c3aa96a5f9f160d90dd95402ad578e2b0) pulled in, and it builds just fine for me: axboe@r7525 ~/gi/build (test)> make clean 9.827s HOSTCC scripts/basic/fixdep HOSTCC scripts/kconfig/conf.o HOSTCC scripts/kconfig/confdata.o HOSTCC scripts/kconfig/expr.o LEX scripts/kconfig/lexer.lex.c YACC scripts/kconfig/parser.tab.[ch] HOSTCC scripts/kconfig/lexer.lex.o HOSTCC scripts/kconfig/menu.o HOSTCC scripts/kconfig/parser.tab.o HOSTCC scripts/kconfig/preprocess.o HOSTCC scripts/kconfig/symbol.o HOSTCC scripts/kconfig/util.o HOSTLD scripts/kconfig/conf # # No change to .config # axboe@r7525 ~/gi/build (test)> time make -j256 -s
On 8/12/22 3:01 PM, Jens Axboe wrote: > On 8/12/22 2:44 PM, Jens Axboe wrote: >> On Aug 12, 2022, at 2:28 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: >>> >>> ?On Fri, Aug 12, 2022 at 5:46 AM Jens Axboe <axboe@kernel.dk> wrote: >>>> >>>> - Small series improving type safety of the sqe fields (Stefan) >>> >>> This doesn't work AT ALL. >>> >>> A basic allmodconfig build fails with tons of errors. It starts with >>> >>> In function ?io_kiocb_cmd_sz_check?, >>> inlined from ?io_prep_rw? at io_uring/rw.c:38:21: >>> ././include/linux/compiler_types.h:354:45: error: call to >>> ?__compiletime_assert_802? declared with attribute error: BUILD_BUG_ON >>> failed: cmd_sz > sizeof(struct io_cmd_data) >>> 354 | _compiletime_assert(condition, msg, >>> __compiletime_assert_, __COUNTER__) >>> | ^ >>> ././include/linux/compiler_types.h:335:25: note: in definition of >>> macro ?__compiletime_assert? >>> 335 | prefix ## suffix(); >>> \ >>> | ^~~~~~ >>> ././include/linux/compiler_types.h:354:9: note: in expansion of >>> macro ?_compiletime_assert? >>> 354 | _compiletime_assert(condition, msg, >>> __compiletime_assert_, __COUNTER__) >>> | ^~~~~~~~~~~~~~~~~~~ >>> ./include/linux/build_bug.h:39:37: note: in expansion of macro >>> ?compiletime_assert? >>> 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) >>> | ^~~~~~~~~~~~~~~~~~ >>> ./include/linux/build_bug.h:50:9: note: in expansion of macro >>> ?BUILD_BUG_ON_MSG? >>> 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " >>> #condition) >>> | ^~~~~~~~~~~~~~~~ >>> ./include/linux/io_uring_types.h:496:9: note: in expansion of macro >>> ?BUILD_BUG_ON? >>> 496 | BUILD_BUG_ON(cmd_sz > sizeof(struct io_cmd_data)); >>> | ^~~~~~~~~~~~ >>> >>> and goes downhill from there. >>> >>> I don't think this can have seen any testing at all. >> >> Wtf? I always run allmodconfig before sending and it also ran testing. >> I?ll check shortly. Sorry about that, whatever went wrong here. > > My test box is still on the same sha from this morning, which is: > > commit 2ae08b36c06ea8df73a79f6b80ff7964e006e9e3 (origin/master, origin/HEAD) > Merge: 21f9c8a13bb2 8bb5e7f4dcd9 > Author: Linus Torvalds <torvalds@linux-foundation.org> > Date: Thu Aug 11 09:23:08 2022 -0700 > > Merge tag 'input-for-v5.20-rc0' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input > > with io_uring-6.0 (ff34d8d06a1f16b6a58fb41bfbaa475cc6c02497) and > block-6.0 (aa0c680c3aa96a5f9f160d90dd95402ad578e2b0) pulled in, and it > builds just fine for me: > > axboe@r7525 ~/gi/build (test)> make clean 9.827s > HOSTCC scripts/basic/fixdep > HOSTCC scripts/kconfig/conf.o > HOSTCC scripts/kconfig/confdata.o > HOSTCC scripts/kconfig/expr.o > LEX scripts/kconfig/lexer.lex.c > YACC scripts/kconfig/parser.tab.[ch] > HOSTCC scripts/kconfig/lexer.lex.o > HOSTCC scripts/kconfig/menu.o > HOSTCC scripts/kconfig/parser.tab.o > HOSTCC scripts/kconfig/preprocess.o > HOSTCC scripts/kconfig/symbol.o > HOSTCC scripts/kconfig/util.o > HOSTLD scripts/kconfig/conf > # > # No change to .config > # > axboe@r7525 ~/gi/build (test)> time make -j256 -s > > ________________________________________________________ > Executed in 172.67 secs fish external > usr time 516.61 mins 396.00 micros 516.61 mins > sys time 44.40 mins 0.00 micros 44.40 mins > > using: > > axboe@r7525 ~/gi/build (test)> gcc --version > gcc (Debian 12.1.0-7) 12.1.0 > > Puzzled, I'll keep poking... I re-did an allmodconfig, and also built on arm64, and I have to say I'm puzzled with what you are seeing. Updated to latest master as well, nothing. Furthermore, I have the build bot send me successful build notifications as well, not just the errors, and here's what it reported 12h ago: tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git io_uring-6.0 branch HEAD: ff34d8d06a1f16b6a58fb41bfbaa475cc6c02497 io_uring: add missing BUILD_BUG_ON() checks for new io_uring_sqe fields elapsed time: 882m configs tested: 53 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: um x86_64_defconfig um i386_defconfig i386 defconfig i386 randconfig-a014 i386 randconfig-a012 i386 randconfig-a016 x86_64 randconfig-a013 x86_64 randconfig-a011 arc randconfig-r043-20220811 arm defconfig x86_64 randconfig-a015 m68k allmodconfig arc allyesconfig i386 allyesconfig i386 randconfig-a001 x86_64 rhel-8.3-func i386 randconfig-a003 x86_64 defconfig alpha allyesconfig mips allyesconfig arm allyesconfig x86_64 rhel-8.3-kunit m68k allyesconfig powerpc allnoconfig arm64 allyesconfig i386 randconfig-a005 x86_64 rhel-8.3 sh allmodconfig x86_64 rhel-8.3-kvm x86_64 allyesconfig x86_64 rhel-8.3-kselftests x86_64 rhel-8.3-syz x86_64 randconfig-a002 x86_64 randconfig-a004 x86_64 randconfig-a006 ia64 allmodconfig powerpc allmodconfig clang tested configs: i386 randconfig-a011 i386 randconfig-a013 hexagon randconfig-r045-20220811 hexagon randconfig-r041-20220811 x86_64 randconfig-a012 s390 randconfig-r044-20220811 i386 randconfig-a015 riscv randconfig-r042-20220811 x86_64 randconfig-a014 x86_64 randconfig-a016 i386 randconfig-a002 i386 randconfig-a006 i386 randconfig-a004 x86_64 randconfig-a001 x86_64 randconfig-a003 x86_64 randconfig-a005 building that very same sha I sent you. WTF? I assure you this thing has been both built, and not just by me, and runtime tested as per usual. Why it fails on your end, I really have no good clue right now.
On 8/12/22 3:08 PM, Jens Axboe wrote: > On 8/12/22 3:01 PM, Jens Axboe wrote: >> On 8/12/22 2:44 PM, Jens Axboe wrote: >>> On Aug 12, 2022, at 2:28 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: >>>> >>>> ?On Fri, Aug 12, 2022 at 5:46 AM Jens Axboe <axboe@kernel.dk> wrote: >>>>> >>>>> - Small series improving type safety of the sqe fields (Stefan) >>>> >>>> This doesn't work AT ALL. >>>> >>>> A basic allmodconfig build fails with tons of errors. It starts with >>>> >>>> In function ?io_kiocb_cmd_sz_check?, >>>> inlined from ?io_prep_rw? at io_uring/rw.c:38:21: >>>> ././include/linux/compiler_types.h:354:45: error: call to >>>> ?__compiletime_assert_802? declared with attribute error: BUILD_BUG_ON >>>> failed: cmd_sz > sizeof(struct io_cmd_data) >>>> 354 | _compiletime_assert(condition, msg, >>>> __compiletime_assert_, __COUNTER__) >>>> | ^ >>>> ././include/linux/compiler_types.h:335:25: note: in definition of >>>> macro ?__compiletime_assert? >>>> 335 | prefix ## suffix(); >>>> \ >>>> | ^~~~~~ >>>> ././include/linux/compiler_types.h:354:9: note: in expansion of >>>> macro ?_compiletime_assert? >>>> 354 | _compiletime_assert(condition, msg, >>>> __compiletime_assert_, __COUNTER__) >>>> | ^~~~~~~~~~~~~~~~~~~ >>>> ./include/linux/build_bug.h:39:37: note: in expansion of macro >>>> ?compiletime_assert? >>>> 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) >>>> | ^~~~~~~~~~~~~~~~~~ >>>> ./include/linux/build_bug.h:50:9: note: in expansion of macro >>>> ?BUILD_BUG_ON_MSG? >>>> 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " >>>> #condition) >>>> | ^~~~~~~~~~~~~~~~ >>>> ./include/linux/io_uring_types.h:496:9: note: in expansion of macro >>>> ?BUILD_BUG_ON? >>>> 496 | BUILD_BUG_ON(cmd_sz > sizeof(struct io_cmd_data)); >>>> | ^~~~~~~~~~~~ >>>> >>>> and goes downhill from there. >>>> >>>> I don't think this can have seen any testing at all. >>> >>> Wtf? I always run allmodconfig before sending and it also ran testing. >>> I?ll check shortly. Sorry about that, whatever went wrong here. >> >> My test box is still on the same sha from this morning, which is: >> >> commit 2ae08b36c06ea8df73a79f6b80ff7964e006e9e3 (origin/master, origin/HEAD) >> Merge: 21f9c8a13bb2 8bb5e7f4dcd9 >> Author: Linus Torvalds <torvalds@linux-foundation.org> >> Date: Thu Aug 11 09:23:08 2022 -0700 >> >> Merge tag 'input-for-v5.20-rc0' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input >> >> with io_uring-6.0 (ff34d8d06a1f16b6a58fb41bfbaa475cc6c02497) and >> block-6.0 (aa0c680c3aa96a5f9f160d90dd95402ad578e2b0) pulled in, and it >> builds just fine for me: >> >> axboe@r7525 ~/gi/build (test)> make clean 9.827s >> HOSTCC scripts/basic/fixdep >> HOSTCC scripts/kconfig/conf.o >> HOSTCC scripts/kconfig/confdata.o >> HOSTCC scripts/kconfig/expr.o >> LEX scripts/kconfig/lexer.lex.c >> YACC scripts/kconfig/parser.tab.[ch] >> HOSTCC scripts/kconfig/lexer.lex.o >> HOSTCC scripts/kconfig/menu.o >> HOSTCC scripts/kconfig/parser.tab.o >> HOSTCC scripts/kconfig/preprocess.o >> HOSTCC scripts/kconfig/symbol.o >> HOSTCC scripts/kconfig/util.o >> HOSTLD scripts/kconfig/conf >> # >> # No change to .config >> # >> axboe@r7525 ~/gi/build (test)> time make -j256 -s >> >> ________________________________________________________ >> Executed in 172.67 secs fish external >> usr time 516.61 mins 396.00 micros 516.61 mins >> sys time 44.40 mins 0.00 micros 44.40 mins >> >> using: >> >> axboe@r7525 ~/gi/build (test)> gcc --version >> gcc (Debian 12.1.0-7) 12.1.0 >> >> Puzzled, I'll keep poking... > > I re-did an allmodconfig, and also built on arm64, and I have to say I'm > puzzled with what you are seeing. Updated to latest master as well, > nothing. Furthermore, I have the build bot send me successful build > notifications as well, not just the errors, and here's what it reported > 12h ago: > > tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git io_uring-6.0 > branch HEAD: ff34d8d06a1f16b6a58fb41bfbaa475cc6c02497 io_uring: add missing BUILD_BUG_ON() checks for new io_uring_sqe fields > > elapsed time: 882m > > configs tested: 53 > configs skipped: 2 > > The following configs have been built successfully. > More configs may be tested in the coming days. > > gcc tested configs: > um x86_64_defconfig > um i386_defconfig > i386 defconfig > i386 randconfig-a014 > i386 randconfig-a012 > i386 randconfig-a016 > x86_64 randconfig-a013 > x86_64 randconfig-a011 > arc randconfig-r043-20220811 > arm defconfig > x86_64 randconfig-a015 > m68k allmodconfig > arc allyesconfig > i386 allyesconfig > i386 randconfig-a001 > x86_64 rhel-8.3-func > i386 randconfig-a003 > x86_64 defconfig > alpha allyesconfig > mips allyesconfig > arm allyesconfig > x86_64 rhel-8.3-kunit > m68k allyesconfig > powerpc allnoconfig > arm64 allyesconfig > i386 randconfig-a005 > x86_64 rhel-8.3 > sh allmodconfig > x86_64 rhel-8.3-kvm > x86_64 allyesconfig > x86_64 rhel-8.3-kselftests > x86_64 rhel-8.3-syz > x86_64 randconfig-a002 > x86_64 randconfig-a004 > x86_64 randconfig-a006 > ia64 allmodconfig > powerpc allmodconfig > > clang tested configs: > i386 randconfig-a011 > i386 randconfig-a013 > hexagon randconfig-r045-20220811 > hexagon randconfig-r041-20220811 > x86_64 randconfig-a012 > s390 randconfig-r044-20220811 > i386 randconfig-a015 > riscv randconfig-r042-20220811 > x86_64 randconfig-a014 > x86_64 randconfig-a016 > i386 randconfig-a002 > i386 randconfig-a006 > i386 randconfig-a004 > x86_64 randconfig-a001 > x86_64 randconfig-a003 > x86_64 randconfig-a005 > > building that very same sha I sent you. WTF? I assure you this thing has > been both built, and not just by me, and runtime tested as per usual. > Why it fails on your end, I really have no good clue right now. Keith brought up a good point, is this some weird randomization of io_kiocb that makes it bigger? struct io_rw is already at 64-bytes as it is, if it gets re-arranged for more padding maybe that's what you're hitting? Is it just io_rw or are you seeing others?
On Fri, Aug 12, 2022 at 2:09 PM Jens Axboe <axboe@kernel.dk> wrote: > > I re-did an allmodconfig, and also built on arm64, Note, while I now do build on my arm64 setup a couple of times daily, this was just the regular x86-64 allmodconfig. > and I have to say I'm puzzled with what you are seeing. > Updated to latest master as well, nothing. I've done "git clean" on my tree and re-did the merge, and will see if it goes away. I *do* have things like the gcc plugins enabled, and I could imagine that randstruct might possibly make structures have different sizes. And those kinds of things get reset by "git clean" when it generates a new seed.. Linus
[ Crossed emails ] On Fri, Aug 12, 2022 at 2:34 PM Jens Axboe <axboe@kernel.dk> wrote: > > Keith brought up a good point, is this some weird randomization of > io_kiocb that makes it bigger? struct io_rw is already at 64-bytes as it > is, if it gets re-arranged for more padding maybe that's what you're > hitting? Is it just io_rw or are you seeing others? I think was seeing others (I got hundreds of lines or errors), but now that I've blown things away I can't recreate it. My allmodconfig build just completed with no sign of the errors I saw earlier. I think Keith is right. An allmodconfig build for me has CONFIG_RANDSTRUCT=y CONFIG_GCC_PLUGIN_RANDSTRUCT=y and the io_uring "type safety" isn't actually typesafe: it just checks the size of types. The other alternative is that we have some build dependency issue, and blowing away my old tree fixed things. But that sounds unlikely, we haven't had those kinds of issues in a long time. Linus
On 8/12/22 3:43 PM, Linus Torvalds wrote: > [ Crossed emails ] > > On Fri, Aug 12, 2022 at 2:34 PM Jens Axboe <axboe@kernel.dk> wrote: >> >> Keith brought up a good point, is this some weird randomization of >> io_kiocb that makes it bigger? struct io_rw is already at 64-bytes as it >> is, if it gets re-arranged for more padding maybe that's what you're >> hitting? Is it just io_rw or are you seeing others? > > I think was seeing others (I got hundreds of lines or errors), but now > that I've blown things away I can't recreate it. My allmodconfig build > just completed with no sign of the errors I saw earlier. > > I think Keith is right. An allmodconfig build for me has > > CONFIG_RANDSTRUCT=y > CONFIG_GCC_PLUGIN_RANDSTRUCT=y > > and the io_uring "type safety" isn't actually typesafe: it just checks > the size of types. I think Keith is right too... > The other alternative is that we have some build dependency issue, and > blowing away my old tree fixed things. But that sounds unlikely, we > haven't had those kinds of issues in a long time. I would've said we just revert it, but this looks broken now with the io_cmd_data layout. Before this release, it just would've grown io_kiocb a bit and spilled into the next cacheline for per-command data. And while that isn't ideal for performance reasons, it's not like it wouldn't work. But now we have it specifically set aside 64 bytes for command data, where it really should be the max any command type would use rather than a hard-coded 64 bytes. I suspect io_rw is the only one that'd hit this, exactly because of the randomization in struct kiocb. If anything else went beyond, we'd find it during development, not while it was in-tree. I'll ponder a solution to this...
On Fri, Aug 12, 2022 at 2:43 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > I think was seeing others (I got hundreds of lines or errors), but now > that I've blown things away I can't recreate it. My allmodconfig build > just completed with no sign of the errors I saw earlier. Oh, and immediately after sending that email, I got the errors back. Because of the randstruct issue, I did another "git clean" (to make sure the random seed was gone and recreated) and started a new allmodconfig build. And now I see the error again. It does seem to be only 'struct io_cmd_data', but since this seems to be about random layout, who knows. The "hundreds of lines" is because each error report ends up being something like 25 lines in size, so you don't need a lot of them to get lots and lots of lines. The ones I see in my *current* build are all that 496 | BUILD_BUG_ON(cmd_sz > sizeof(struct io_cmd_data)); add there's apparently six of them (so the "hundreds of lines" was apparently "only" 150 lines of errors), here's the concise "inlined from" info: inlined from ‘io_prep_rw’ at io_uring/rw.c:38:21: inlined from ‘__io_import_iovec’ at io_uring/rw.c:353:21, inlined from ‘io_import_iovec’ at io_uring/rw.c:406:11, inlined from ‘io_rw_prep_async’ at io_uring/rw.c:538:8, inlined from ‘io_readv_prep_async’ at io_uring/rw.c:551:9: inlined from ‘io_read’ at io_uring/rw.c:697:21: inlined from ‘io_write’ at io_uring/rw.c:842:21: inlined from ‘io_do_iopoll’ at io_uring/rw.c:997:22: in case that helps. Anyway, the way to recreate this is apparently (a) make sure you have the gcc plugins enabled (b) do "git clean -dqfx" between builds (c) do "make allmodconfig ; make -j64" until you see it. I don't know what the chance of it happening is, but I'm starting my fourth iteration, and I am just now seen it for the third time. So it seems fairly easy to trigger, even if it's not 100%. And yes, on this latest case it was once again "struct io_cmd_data". So I think that may be the only case. Linus
On Fri, Aug 12, 2022 at 2:54 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > And yes, on this latest case it was once again "struct io_cmd_data". Duh. I'm a nincompoop. I'm only looking at the BUG_ON(), but io_cmd_data is the *good* case that should cover it all, the "cmd_sz" is the problem case, and the problematic stricture name doesn't actually show up in the BUILD_BUG_ON() output. So you have to look at where it's inlined from and check them individually, and yeah, it seems to be 'struct io_rw' every time. Linus
On 8/12/22 3:54 PM, Linus Torvalds wrote: > On Fri, Aug 12, 2022 at 2:43 PM Linus Torvalds > <torvalds@linux-foundation.org> wrote: >> >> I think was seeing others (I got hundreds of lines or errors), but now >> that I've blown things away I can't recreate it. My allmodconfig build >> just completed with no sign of the errors I saw earlier. > > Oh, and immediately after sending that email, I got the errors back. > > Because of the randstruct issue, I did another "git clean" (to make > sure the random seed was gone and recreated) and started a new > allmodconfig build. > > And now I see the error again. > > It does seem to be only 'struct io_cmd_data', but since this seems to > be about random layout, who knows. The "hundreds of lines" is because > each error report ends up being something like 25 lines in size, so > you don't need a lot of them to get lots and lots of lines. > > The ones I see in my *current* build are all that > > 496 | BUILD_BUG_ON(cmd_sz > sizeof(struct io_cmd_data)); > > add there's apparently six of them (so the "hundreds of lines" was > apparently "only" 150 lines of errors), here's the concise "inlined > from" info: > > inlined from ?io_prep_rw? at io_uring/rw.c:38:21: > inlined from ?__io_import_iovec? at io_uring/rw.c:353:21, > inlined from ?io_import_iovec? at io_uring/rw.c:406:11, > inlined from ?io_rw_prep_async? at io_uring/rw.c:538:8, > inlined from ?io_readv_prep_async? at io_uring/rw.c:551:9: > inlined from ?io_read? at io_uring/rw.c:697:21: > inlined from ?io_write? at io_uring/rw.c:842:21: > inlined from ?io_do_iopoll? at io_uring/rw.c:997:22: > > in case that helps. Thanks it does - so it's just io_rw, and hence it's just kiocb that is problematic because of that layout randomization. What we really want is for: struct io_cmd_data { struct file *file; /* each command gets 56 bytes of data */ __u8 data[56]; }; to be == max-of-any-request-type data. Which was 56 bytes before, io_rw and a few others were at that size. But if kiocb changes in an unlucky fashion and we get the u16 and int interspersed between different types, then struct kiocb growns and then io_rw as a result. And then the compile blows up. The patch was obviously a good thing since it found this, as this would've caused some weird breakage that would've been hard to reproduce unless your own build ended up having kiocb be larger as well. Question is what to do about it. I can't think of a good way to size io_cmd_data appropriately. We can union an io_rw in there, since that's the biggest one and I _think_ the only one that'd hit this due to the randomization. If I'm wrong, then it'd break compile again obviously. Or we can ensure that kiocb doesn't get re-organized such that we add more holes/padding. But that's also kind of weird. Ideally we'd have a compile time way to check all structs, but that becomes unwieldy. For that one suggestion, I suspect this will fix your issue. It's obviously not a thing of beauty... diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h index 677a25d44d7f..c83dedeb44b9 100644 --- a/include/linux/io_uring_types.h +++ b/include/linux/io_uring_types.h @@ -481,14 +481,29 @@ struct io_cqe { }; }; +struct io_rw { + /* NOTE: kiocb has the file as the first member, so don't do it here */ + struct kiocb kiocb; + u64 addr; + u32 len; + rwf_t flags; +}; + /* * Each request type overlays its private data structure on top of this one. - * They must not exceed this one in size. + * They must not exceed this one in size. We must ensure that this is big + * enough to hold any command type. Currently io_rw includes struct kiocb, + * which is marked as having a random layout for security reasons. This can + * cause it to grow in size if the layout ends up adding more holes or padding. + * Unionize io_cmd_data with io_rw to work-around this issue. */ struct io_cmd_data { - struct file *file; - /* each command gets 56 bytes of data */ - __u8 data[56]; + union { + struct file *file; + /* each command gets 56 bytes of data */ + __u8 data[56]; + }; + struct io_rw pad; }; static inline void io_kiocb_cmd_sz_check(size_t cmd_sz) diff --git a/io_uring/rw.c b/io_uring/rw.c index 1babd77da79c..1c3a5da9dcdc 100644 --- a/io_uring/rw.c +++ b/io_uring/rw.c @@ -20,14 +20,6 @@ #include "rsrc.h" #include "rw.h" -struct io_rw { - /* NOTE: kiocb has the file as the first member, so don't do it here */ - struct kiocb kiocb; - u64 addr; - u32 len; - rwf_t flags; -}; - static inline bool io_file_supports_nowait(struct io_kiocb *req) { return req->flags & REQ_F_SUPPORT_NOWAIT;
On 8/12/22 4:01 PM, Linus Torvalds wrote: > On Fri, Aug 12, 2022 at 2:54 PM Linus Torvalds > <torvalds@linux-foundation.org> wrote: >> >> And yes, on this latest case it was once again "struct io_cmd_data". > > Duh. I'm a nincompoop. I'm only looking at the BUG_ON(), but > io_cmd_data is the *good* case that should cover it all, the "cmd_sz" > is the problem case, and the problematic stricture name doesn't > actually show up in the BUILD_BUG_ON() output. > > So you have to look at where it's inlined from and check them > individually, and yeah, it seems to be 'struct io_rw' every time. Right, see my reply - it's struct io_rw because of the kiocb changing size depending on what layout is picked. I sent a hack to fix it in that email. I'm _pretty_ sure this is only io_rw as we generally don't include a lot of kernel structs in per-command structs. So while it's a bit of an eye sore (and moves io_rw into the public domain rather than be rw.c private), it should work around the issue.
On 8/12/22 4:11 PM, Jens Axboe wrote: > For that one suggestion, I suspect this will fix your issue. It's > obviously not a thing of beauty... While it did fix compile, it's also wrong obviously as io_rw needs to be in that union... Thanks Keith, again! diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h index 677a25d44d7f..7ef7cffff0d2 100644 --- a/include/linux/io_uring_types.h +++ b/include/linux/io_uring_types.h @@ -481,14 +481,31 @@ struct io_cqe { }; }; +struct io_rw { + /* NOTE: kiocb has the file as the first member, so don't do it here */ + struct kiocb kiocb; + u64 addr; + u32 len; + rwf_t flags; +}; + /* * Each request type overlays its private data structure on top of this one. - * They must not exceed this one in size. + * They must not exceed this one in size. We must ensure that this is big + * enough to hold any command type. Currently io_rw includes struct kiocb, + * which is marked as having a random layout for security reasons. This can + * cause it to grow in size if the layout ends up adding more holes or padding. + * Unionize io_cmd_data with io_rw to work-around this issue. */ struct io_cmd_data { - struct file *file; - /* each command gets 56 bytes of data */ - __u8 data[56]; + union { + struct { + struct file *file; + /* each command gets 56 bytes of data */ + __u8 data[56]; + }; + struct io_rw pad; + }; }; static inline void io_kiocb_cmd_sz_check(size_t cmd_sz) diff --git a/io_uring/rw.c b/io_uring/rw.c index 1babd77da79c..1c3a5da9dcdc 100644 --- a/io_uring/rw.c +++ b/io_uring/rw.c @@ -20,14 +20,6 @@ #include "rsrc.h" #include "rw.h" -struct io_rw { - /* NOTE: kiocb has the file as the first member, so don't do it here */ - struct kiocb kiocb; - u64 addr; - u32 len; - rwf_t flags; -}; - static inline bool io_file_supports_nowait(struct io_kiocb *req) { return req->flags & REQ_F_SUPPORT_NOWAIT;
On Fri, Aug 12, 2022 at 04:19:06PM -0600, Jens Axboe wrote: > On 8/12/22 4:11 PM, Jens Axboe wrote: > > For that one suggestion, I suspect this will fix your issue. It's > > obviously not a thing of beauty... > > While it did fix compile, it's also wrong obviously as io_rw needs to be > in that union... Thanks Keith, again! I'd prefer if we can get away with forcing struct kiocb to not grow. The below should have the randomization move the smallest two fields together so we don't introduce more padding than necessary: --- diff --git a/include/linux/fs.h b/include/linux/fs.h index 5113f65c786f..ef7b277cb7f3 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -347,8 +347,10 @@ struct kiocb { loff_t ki_pos; void (*ki_complete)(struct kiocb *iocb, long ret); void *private; - int ki_flags; - u16 ki_ioprio; /* See linux/ioprio.h */ + struct { + int ki_flags; + u16 ki_ioprio; /* See linux/ioprio.h */ + }; struct wait_page_queue *ki_waitq; /* for async buffered IO */ randomized_struct_fields_end }; --
On 8/12/22 4:23 PM, Keith Busch wrote: > On Fri, Aug 12, 2022 at 04:19:06PM -0600, Jens Axboe wrote: >> On 8/12/22 4:11 PM, Jens Axboe wrote: >>> For that one suggestion, I suspect this will fix your issue. It's >>> obviously not a thing of beauty... >> >> While it did fix compile, it's also wrong obviously as io_rw needs to be >> in that union... Thanks Keith, again! > > I'd prefer if we can get away with forcing struct kiocb to not grow. > The below should have the randomization move the smallest two fields > together so we don't introduce more padding than necessary: Keith suggested the same thing and was going to send it out, and I definitely like that a lot more than mine. Can you commit something like this? Should probably add a comment on why these are grouped like that, though.
On 8/12/22 4:25 PM, Jens Axboe wrote: > On 8/12/22 4:23 PM, Keith Busch wrote: >> On Fri, Aug 12, 2022 at 04:19:06PM -0600, Jens Axboe wrote: >>> On 8/12/22 4:11 PM, Jens Axboe wrote: >>>> For that one suggestion, I suspect this will fix your issue. It's >>>> obviously not a thing of beauty... >>> >>> While it did fix compile, it's also wrong obviously as io_rw needs to be >>> in that union... Thanks Keith, again! >> >> I'd prefer if we can get away with forcing struct kiocb to not grow. >> The below should have the randomization move the smallest two fields >> together so we don't introduce more padding than necessary: > > Keith suggested the same thing and was going to send it out, and I > definitely like that a lot more than mine. Can you commit something like > this? Should probably add a comment on why these are grouped like that, > though. Guess I'm a bit too quick here as I read this one as being from Linus. Linus, what do you think of this? It'll prevent kiocb from growing due to fields being moved around.
On Fri, Aug 12, 2022 at 3:24 PM Keith Busch <kbusch@kernel.org> wrote: > > I'd prefer if we can get away with forcing struct kiocb to not grow. The below > should have the randomization move the smallest two fields together so we don't > introduce more padding than necessary: I like this concept, but I think you might hit issues on 32-bit, where "loff_t" can be a 64-bit entity with 64-bit alignment, and then you see the same "oops, now it grew because loff_t moved" there. Note that you'd never see that on x86-32, because - for strange historical reasons - even 64-bit fields only have 32-bit alignment, but other 32-bit architectures don't act that way. Honestly, I think maybe we should just stop randomizing kiocb. Because it has six fields, and basically three of them we wouldn't want to randomly move around. At some point, randomization no longer even matters. Linus
On 8/12/22 4:35 PM, Linus Torvalds wrote:
> Honestly, I think maybe we should just stop randomizing kiocb.
That'd obviously solve it... Do you want to commit something like that?
Or would you prefer a patch to do so?
On Fri, Aug 12, 2022 at 3:38 PM Jens Axboe <axboe@kernel.dk> wrote: > > On 8/12/22 4:35 PM, Linus Torvalds wrote: > > Honestly, I think maybe we should just stop randomizing kiocb. > > That'd obviously solve it... Do you want to commit something like that? > Or would you prefer a patch to do so? Please send an updated io_uring pull with that included, Linus
On 8/12/22 4:52 PM, Linus Torvalds wrote: > On Fri, Aug 12, 2022 at 3:38 PM Jens Axboe <axboe@kernel.dk> wrote: >> >> On 8/12/22 4:35 PM, Linus Torvalds wrote: >>> Honestly, I think maybe we should just stop randomizing kiocb. >> >> That'd obviously solve it... Do you want to commit something like that? >> Or would you prefer a patch to do so? > > Please send an updated io_uring pull with that included, OK will do, I'll reorder to avoid the breakage too.
Hi Linus, A few fixes that should go upstream before 6.0-rc1. In detail: - Regression fix for this merge window, fixing a wrong order of arguments for io_req_set_res() for passthru (Dylan) - Fix for the audit code leaking context memory (Peilin) - Ensure that provided buffers are memcg accounted (Pavel) - Correctly handle short zero-copy sends (Pavel) - Sparse warning fixes for the recvmsg multishot command (Dylan) - Small series improving type safety of the sqe fields (Stefan) - Error handling fix for passthru (Anuj) Please pull! The following changes since commit e2b542100719a93f8cdf6d90185410d38a57a4c1: Merge tag 'flexible-array-transformations-UAPI-6.0-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux (2022-08-02 19:50:47 -0700) are available in the Git repository at: git://git.kernel.dk/linux-block.git tags/io_uring-6.0-2022-08-12 for you to fetch changes up to ff34d8d06a1f16b6a58fb41bfbaa475cc6c02497: io_uring: add missing BUILD_BUG_ON() checks for new io_uring_sqe fields (2022-08-11 10:56:13 -0600) ---------------------------------------------------------------- io_uring-6.0-2022-08-12 ---------------------------------------------------------------- Anuj Gupta (1): io_uring: fix error handling for io_uring_cmd Dylan Yudaken (1): io_uring: fix io_recvmsg_prep_multishot sparse warnings Ming Lei (1): io_uring: pass correct parameters to io_req_set_res Pavel Begunkov (2): io_uring: mem-account pbuf buckets io_uring/net: send retry for zerocopy Peilin Ye (1): audit, io_uring, io-wq: Fix memory leak in io_sq_thread() and io_wqe_worker() Stefan Metzmacher (3): io_uring: consistently make use of io_notif_to_data() io_uring: make io_kiocb_to_cmd() typesafe io_uring: add missing BUILD_BUG_ON() checks for new io_uring_sqe fields include/linux/audit.h | 5 ---- include/linux/io_uring_types.h | 9 +++++- io_uring/advise.c | 8 ++--- io_uring/cancel.c | 4 +-- io_uring/epoll.c | 4 +-- io_uring/fs.c | 28 +++++++++--------- io_uring/io-wq.c | 3 -- io_uring/io_uring.c | 19 ++++++++++-- io_uring/kbuf.c | 10 +++---- io_uring/msg_ring.c | 8 ++--- io_uring/net.c | 66 +++++++++++++++++++++++++----------------- io_uring/notif.c | 4 +-- io_uring/notif.h | 2 +- io_uring/openclose.c | 16 +++++----- io_uring/poll.c | 16 +++++----- io_uring/rsrc.c | 10 +++---- io_uring/rw.c | 28 +++++++++--------- io_uring/splice.c | 8 ++--- io_uring/sqpoll.c | 4 --- io_uring/statx.c | 6 ++-- io_uring/sync.c | 12 ++++---- io_uring/timeout.c | 26 ++++++++--------- io_uring/uring_cmd.c | 17 +++++++---- io_uring/xattr.c | 18 ++++++------ kernel/auditsc.c | 25 ---------------- 25 files changed, 178 insertions(+), 178 deletions(-)