block: Replace bio_check_ro()'s WARN_ON()
PATCH 1/1 block: Replace bio_check_ro()'s WARN_ON()
As described in commit 96c6a32ccb55a ("include/asm-generic/bug.h: clarify valid uses of WARN()"), this replaces a userspace-reachable WARN_ON() with pr_warn_once(). The reachability is even noted in the existing comment. This appears to be an "expected by unlikely" condition, so getting rid of the WARN_ON() means kernel fuzzers will stop reporting the problem. Additionally un-breaks the error string so it can more easily be found with grep.
--- block/blk-core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
block/blk-core.c +2 -4
@@ -2166,11 +2166,9 @@ static inline bool bio_check_ro(struct bio *bio, struct hd_struct *part)if (part->policy && (op_is_write (op) && !op_is_flush (op))) {char b[BDEVNAME_SIZE ]; -WARN_ONCE (1 , -"generic_make_request: Trying to write " -"to read-only block-device %s (partno %d)\n" , +/* Older lvm-tools actually triggers this. */ +pr_warn_once ("Trying to write to read-only block-device %s (partno %d)\n" ,bio_devname (bio, b), part->partno); -/* Older lvm-tools actually trigger this */ return false; }
Signature · 6 lines
2.17.1 -- Kees Cook Pixel Security
Friendly ping! I keep tripping over this. Can this please get applied so we can silence syzbot and avoid needless WARNs? :)
-Kees
Quoted 41 lines · Kees Cook
As described in commit
96c6a32ccb55a("include/asm-generic/bug.h: clarify valid uses ofWARN()"), this replaces a userspace-reachableWARN_ON()withpr_warn_once(). The reachability is even noted in the existing comment. This appears to be an "expected by unlikely" condition, so getting rid of theWARN_ON()means kernel fuzzers will stop reporting the problem. Additionally un-breaks the error string so it can more easily be found with grep.Reported-by [email protected]Cc Jens Axboe <[email protected]>Signed-off-by Kees Cook <[email protected]>--- block/blk-core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)block/blk-core.c +2
-4@@ -2166,11 +2166,9 @@ static inline bool bio_check_ro(struct bio *bio, struct hd_struct *part)if (part->policy && (op_is_write (op) && !op_is_flush (op))) {char b[BDEVNAME_SIZE ]; -WARN_ONCE (1 , -"generic_make_request: Trying to write " -"to read-only block-device %s (partno %d)\n" , +/* Older lvm-tools actually triggers this. */ +pr_warn_once ("Trying to write to read-only block-device %s (partno %d)\n" ,bio_devname (bio, b), part->partno); -/* Older lvm-tools actually trigger this */ return false; }Signature · 6 lines
2.17.1 -- Kees Cook Pixel Security
Signature · 1 line
Kees Cook
Jens Axboe
maintainerQuoted 2 lines · Kees Cook
Friendly ping! I keep tripping over this. Can this please get applied so we can silence syzbot and avoid needless WARNs? :)
I'll get it applied, I did see syzbot complain about this again.
Signature · 1 line
Jens Axboe
Quoted 5 lines · Jens Axboe
Quoted 2 lines · Kees Cook
Friendly ping! I keep tripping over this. Can this please get applied so we can silence syzbot and avoid needless WARNs? :)
I'll get it applied, I did see syzbot complain about this again.
Awesome; thanks! :)
Signature · 1 line
Kees Cook
Quoted 2 lines · Kees Cook
Friendly ping! I keep tripping over this. Can this please get applied so we can silence syzbot and avoid needless WARNs? :)
What call stack reaches this? Upper layers should never submit a write bio on a read-only queue, and we need to fix that in the upper layer.
Jens Axboe
maintainerQuoted 6 lines · Christoph Hellwig
Quoted 2 lines · Kees Cook
Friendly ping! I keep tripping over this. Can this please get applied so we can silence syzbot and avoid needless WARNs? :)
What call stack reaches this? Upper layers should never submit a write bio on a read-only queue, and we need to fix that in the upper layer.
It's an fsync, the trace is here:
https://syzkaller.appspot.com/x/log.txt?x=159503d2e00000
Signature · 1 line
Jens Axboe
Quoted 11 lines · Jens Axboe
Quoted 6 lines · Christoph Hellwig
Quoted 2 lines · Kees Cook
Friendly ping! I keep tripping over this. Can this please get applied so we can silence syzbot and avoid needless WARNs? :)
What call stack reaches this? Upper layers should never submit a write bio on a read-only queue, and we need to fix that in the upper layer.
It's an fsync, the trace is here:
Oh. I think this is a bug in the block layer, we should not treat a sync as write for the purposes of is read-only checks, as it never writes data to the device. At the request layer we alread use the proper REQ_OP_FLUSH, but at the bio layer we are still abusing empty writes apparently. I'll try to cook up something over the weekend.
Quoted 11 lines · Christoph Hellwig
Quoted 10 lines · Jens Axboe
Quoted 6 lines · Christoph Hellwig
Quoted 2 lines · Kees Cook
Friendly ping! I keep tripping over this. Can this please get applied so we can silence syzbot and avoid needless WARNs? :)
What call stack reaches this? Upper layers should never submit a write bio on a read-only queue, and we need to fix that in the upper layer.
It's an fsync, the trace is here:
Quoted 8 lines
Oh. I think this is a bug in the block layer, we should not treat a sync as write for the purposes of is read-only checks, as it never writes data to the device. At the request layer we alread use the proper
REQ_OP_FLUSH, but at the bio layer we are still abusing empty writes apparently. I'll try to cook up something over the weekend.
Cool; thanks! Note that syzbot has a reproducer for it:
https://syzkaller.appspot.com/text?tag=ReproC&x=117ccc8c400000
If that doesn't work for your own testing, you can ask syzbot to test patches itself:
https://goo.gl/tpsmEJ#testing-patches
Signature · 1 line
Kees Cook
So I looked at this a bit, and doing the right thing (TM) will be a little invase and thus not for 5.5.
But the 5.5. queue already has a patch from Mikulas Patocka:8b2ded1c94c ("block: don't warn when doing fsync on read-only devices") which should deal with this issue, and in fact I can't trigger the WARN_ON with Jens' latest tree.