* [PATCH 0/2] nbd: fix sanity check for first_minor
@ 2021-10-21 12:29 Yu Kuai
2021-10-21 12:29 ` [PATCH 1/2] nbd: fix max value for 'first_minor' Yu Kuai
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Yu Kuai @ 2021-10-21 12:29 UTC (permalink / raw)
To: josef, axboe, paskripkin
Cc: linux-block, nbd, linux-kernel, yukuai3, yi.zhang, luomeng12
Yu Kuai (2):
nbd: fix max value for 'first_minor'
nbd: fix possible overflow for 'first_minor' in nbd_dev_add()
drivers/block/nbd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] nbd: fix max value for 'first_minor'
2021-10-21 12:29 [PATCH 0/2] nbd: fix sanity check for first_minor Yu Kuai
@ 2021-10-21 12:29 ` Yu Kuai
2021-10-21 12:29 ` [PATCH 2/2] nbd: fix possible overflow for 'first_minor' in nbd_dev_add() Yu Kuai
2021-10-21 12:35 ` [PATCH 0/2] nbd: fix sanity check for first_minor Pavel Skripkin
2 siblings, 0 replies; 9+ messages in thread
From: Yu Kuai @ 2021-10-21 12:29 UTC (permalink / raw)
To: josef, axboe, paskripkin
Cc: linux-block, nbd, linux-kernel, yukuai3, yi.zhang, luomeng12
commit b1a811633f73 ("block: nbd: add sanity check for first_minor")
checks that 'first_minor' should not be greater than 0xff, which is
wrong. Whitout the commit, the details that when user pass 0x100000,
it ends up create sysfs dir "/sys/block/43:0" are as follows:
nbd_dev_add
disk->first_minor = index << part_shift
-> default part_shift is 5, first_minor is 0x2000000
device_add_disk
ddev->devt = MKDEV(disk->major, disk->first_minor)
-> (0x2b << 20) | (0x2000000) = 0x2b00000
device_add
device_create_sys_dev_entry
format_dev_t
sprintf(buffer, "%u:%u", MAJOR(dev), MINOR(dev));
-> got 43:0
sysfs_create_link -> /sys/block/43:0
By the way, with the wrong fix, when part_shift is the default value,
only 8 ndb devices can be created since 8 << 5 is greater than 0xff.
Since the max bits for 'first_minor' should be the same as what
MKDEV() does, which is 20. Change the upper bound of 'first_minor'
from 0xff to 0xfffff.
Fixes: b1a811633f73 ("block: nbd: add sanity check for first_minor")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/block/nbd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 0d064fab6186..8f2c17a33c5b 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1808,11 +1808,11 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs)
disk->major = NBD_MAJOR;
/* Too big first_minor can cause duplicate creation of
- * sysfs files/links, since first_minor will be truncated to
- * byte in __device_add_disk().
+ * sysfs files/links, since MKDEV() expect that the max bits of
+ * first_minor is 20.
*/
disk->first_minor = index << part_shift;
- if (disk->first_minor > 0xff) {
+ if (disk->first_minor > 0xfffff) {
err = -EINVAL;
goto out_free_idr;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] nbd: fix possible overflow for 'first_minor' in nbd_dev_add()
2021-10-21 12:29 [PATCH 0/2] nbd: fix sanity check for first_minor Yu Kuai
2021-10-21 12:29 ` [PATCH 1/2] nbd: fix max value for 'first_minor' Yu Kuai
@ 2021-10-21 12:29 ` Yu Kuai
2021-10-21 12:35 ` [PATCH 0/2] nbd: fix sanity check for first_minor Pavel Skripkin
2 siblings, 0 replies; 9+ messages in thread
From: Yu Kuai @ 2021-10-21 12:29 UTC (permalink / raw)
To: josef, axboe, paskripkin
Cc: linux-block, nbd, linux-kernel, yukuai3, yi.zhang, luomeng12
If 'part_shift' is not zero, then 'index << part_shift' might
overflow to a value that is not greater than '0xfffff', then sysfs
might complains about duplicate creation.
Fixes: b0d9111a2d53 ("nbd: use an idr to keep track of nbd devices")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/block/nbd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 8f2c17a33c5b..03a10a87500d 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1808,11 +1808,11 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs)
disk->major = NBD_MAJOR;
/* Too big first_minor can cause duplicate creation of
- * sysfs files/links, since MKDEV() expect that the max bits of
- * first_minor is 20.
+ * sysfs files/links, since index << part_shift might overflow, or
+ * MKDEV() expect that the max bits of first_minor is 20.
*/
disk->first_minor = index << part_shift;
- if (disk->first_minor > 0xfffff) {
+ if (disk->first_minor < index || disk->first_minor > 0xfffff) {
err = -EINVAL;
goto out_free_idr;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] nbd: fix sanity check for first_minor
2021-10-21 12:29 [PATCH 0/2] nbd: fix sanity check for first_minor Yu Kuai
2021-10-21 12:29 ` [PATCH 1/2] nbd: fix max value for 'first_minor' Yu Kuai
2021-10-21 12:29 ` [PATCH 2/2] nbd: fix possible overflow for 'first_minor' in nbd_dev_add() Yu Kuai
@ 2021-10-21 12:35 ` Pavel Skripkin
2021-10-21 13:13 ` yukuai (C)
2 siblings, 1 reply; 9+ messages in thread
From: Pavel Skripkin @ 2021-10-21 12:35 UTC (permalink / raw)
To: Yu Kuai, josef, axboe
Cc: linux-block, nbd, linux-kernel, yi.zhang, luomeng12, Christoph Hellwig
On 10/21/21 15:29, Yu Kuai wrote:
> Yu Kuai (2):
> nbd: fix max value for 'first_minor'
> nbd: fix possible overflow for 'first_minor' in nbd_dev_add()
>
> drivers/block/nbd.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
Hi, Yu!
Thank you for the fix, but this wrong check should be just removed,
since root case of wrong sysfs file creation was fixed, as Christoph
said [1]
[1] https://lore.kernel.org/lkml/20211011073556.GA10735@lst.de/
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] nbd: fix sanity check for first_minor
2021-10-21 12:35 ` [PATCH 0/2] nbd: fix sanity check for first_minor Pavel Skripkin
@ 2021-10-21 13:13 ` yukuai (C)
2021-10-21 13:37 ` yukuai (C)
2021-10-21 16:47 ` Pavel Skripkin
0 siblings, 2 replies; 9+ messages in thread
From: yukuai (C) @ 2021-10-21 13:13 UTC (permalink / raw)
To: Pavel Skripkin, josef, axboe
Cc: linux-block, nbd, linux-kernel, yi.zhang, luomeng12, Christoph Hellwig
On 2021/10/21 20:35, Pavel Skripkin wrote:
> On 10/21/21 15:29, Yu Kuai wrote:
>> Yu Kuai (2):
>> nbd: fix max value for 'first_minor'
>> nbd: fix possible overflow for 'first_minor' in nbd_dev_add()
>>
>> drivers/block/nbd.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>
> Hi, Yu!
>
> Thank you for the fix, but this wrong check should be just removed,
> since root case of wrong sysfs file creation was fixed, as Christoph
> said [1]
Hi, Pavel
Thanks for your response, with the root cause fixed, patch 1 is not
needed anymore. However, the overflow case in patch 2 is still
possible.
Does anyone plan to remove the checking?
Thanks,
Kuai
>
>
>
>
> [1] https://lore.kernel.org/lkml/20211011073556.GA10735@lst.de/
>
>
>
> With regards,
> Pavel Skripkin
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] nbd: fix sanity check for first_minor
2021-10-21 13:13 ` yukuai (C)
@ 2021-10-21 13:37 ` yukuai (C)
2021-10-25 13:41 ` yukuai (C)
2021-10-21 16:47 ` Pavel Skripkin
1 sibling, 1 reply; 9+ messages in thread
From: yukuai (C) @ 2021-10-21 13:37 UTC (permalink / raw)
To: Pavel Skripkin, josef, axboe
Cc: linux-block, nbd, linux-kernel, yi.zhang, luomeng12, Christoph Hellwig
On 2021/10/21 21:13, yukuai (C) wrote:
> On 2021/10/21 20:35, Pavel Skripkin wrote:
>> On 10/21/21 15:29, Yu Kuai wrote:
>>> Yu Kuai (2):
>>> nbd: fix max value for 'first_minor'
>>> nbd: fix possible overflow for 'first_minor' in nbd_dev_add()
>>>
>>> drivers/block/nbd.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>
>> Hi, Yu!
>>
>> Thank you for the fix, but this wrong check should be just removed,
>> since root case of wrong sysfs file creation was fixed, as Christoph
>> said [1]
Hi, Christoph
By the way, if we remove the checking, there will be two kernel warnings
when the problem happens. Maybe keeping the checking is better?
[ 19.860640] sysfs: cannot create duplicate filename '/dev/block/43:0'
[ 19.861659] CPU: 1 PID: 872 Comm: modprobe Not tainted
5.15.0-rc6-next-20211020-00001-g2f1
[ 19.863175] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS ?-20190727_0738364
[ 19.865183] Call Trace:
[ 19.866512] <TASK>
[ 19.866863] ? dump_stack_lvl+0x73/0x9f
[ 19.867941] ? dump_stack+0x13/0x1b
[ 19.868475] ? sysfs_warn_dup.cold+0x27/0x45
[ 19.869075] ? sysfs_do_create_link_sd.isra.0+0x131/0x150
[ 19.869818] ? sysfs_create_link+0x29/0x60
[ 19.870459] ? device_add+0xbd6/0xf60
[ 19.871032] ? _printk+0x5f/0x7d
[ 19.871518] ? device_add_disk+0x153/0x5d0
[ 19.872160] ? nbd_dev_add+0x30e/0x470 [nbd]
[ 19.872828] ? 0xffffffffc0060000
[ 19.873332] ? nbd_init+0x1dc/0x1000 [nbd]
[ 19.873924] ? do_one_initcall+0x71/0x3a0
[ 19.874534] ? gcov_event+0x70/0x690
[ 19.875058] ? do_init_module+0xa6/0x350
[ 19.875587] ? load_module+0x2587/0x2720
[ 19.876130] ? kernel_read+0x31/0xb0
[ 19.876638] ? kernel_read_file+0x15a/0x360
[ 19.877271] ? __do_sys_finit_module+0xe5/0x190
[ 19.877951] ? __do_sys_finit_module+0xe5/0x190
[ 19.878563] ? __x64_sys_finit_module+0x1e/0x30
[ 19.879182] ? do_syscall_64+0x35/0x80
[ 19.879700] ? entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 19.880413] </TASK>
[ 19.880806] ------------[ cut here ]------------
[ 19.881502] WARNING: CPU: 1 PID: 872 at block/genhd.c:543
device_add_disk+0x2af/0x5d0
[ 19.882695] Modules linked in: nbd(+)
[ 19.883290] CPU: 1 PID: 872 Comm: modprobe Not tainted
5.15.0-rc6-next-20211020-00001-g2f1
[ 19.884823] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS ?-20190727_0738364
[ 19.886866] RIP: 0010:device_add_disk+0x2af/0x5d0
[ 19.887606] Code: 6e f2 f2 0b 01 49 8b 76 48 48 8b 3d db 03 f3 0b e8
f6 ec a9 ff 48 83 050
[ 19.890456] RSP: 0018:ffffc90000e47c70 EFLAGS: 00010202
[ 19.891293] RAX: 0000000000000000 RBX: 0000000000000000 RCX:
00000000000ae001
[ 19.892274] RDX: 00000000000ac001 RSI: ffffffff91b5906b RDI:
0000000000000000
[ 19.893318] RBP: ffff8881034ad600 R08: 0000000000000000 R09:
ffffffff915e2e69
[ 19.894425] R10: 0000000000000014 R11: 0000000000000001 R12:
00000000ffffffef
[ 19.895544] R13: 0000000000000000 R14: ffff88817d720600 R15:
ffff88817d720648
[ 19.896652] FS: 00007fc08c7a7040(0000) GS:ffff88882f640000(0000)
knlGS:0000000000000000
[ 19.897902] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 19.898768] CR2: 00007fc08bcf0395 CR3: 000000017c467000 CR4:
00000000000006e0
[ 19.899754] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 19.900856] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[ 19.901989] Call Trace:
[ 19.902387] <TASK>
[ 19.902732] nbd_dev_add+0x30e/0x470 [nbd]
[ 19.903395] ? 0xffffffffc0060000
[ 19.903917] nbd_init+0x1dc/0x1000 [nbd]
[ 19.904536] do_one_initcall+0x71/0x3a0
[ 19.905166] ? gcov_event+0x70/0x690
[ 19.905745] do_init_module+0xa6/0x350
[ 19.906351] load_module+0x2587/0x2720
[ 19.906932] ? kernel_read+0x31/0xb0
[ 19.907509] ? kernel_read_file+0x15a/0x360
[ 19.908195] ? __do_sys_finit_module+0xe5/0x190
[ 19.908894] __do_sys_finit_module+0xe5/0x190
[ 19.909591] __x64_sys_finit_module+0x1e/0x30
[ 19.910289] do_syscall_64+0x35/0x80
[ 19.910855] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 19.911652] RIP: 0033:0x7fc08bc7a4e9
[ 19.912231] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00
48 89 f8 48 89 f7 488
[ 19.915026] RSP: 002b:00007fff0ddf6fd8 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[ 19.916080] RAX: ffffffffffffffda RBX: 00005596400853e0 RCX:
00007fc08bc7a4e9
[ 19.917180] RDX: 0000000000000000 RSI: 000055963fe1bc26 RDI:
0000000000000003
[ 19.918302] RBP: 000055963fe1bc26 R08: 0000000000000000 R09:
0000000000000000
[ 19.919412] R10: 0000000000000003 R11: 0000000000000246 R12:
0000000000000000
[ 19.920524] R13: 00005596400854f0 R14: 0000000000040000 R15:
00005596400853e0
[ 19.921629] </TASK>
[ 19.922005] ---[ end trace e09ecf130812479d ]---
Thanks,
Kuai
>
> Hi, Pavel
>
> Thanks for your response, with the root cause fixed, patch 1 is not
> needed anymore. However, the overflow case in patch 2 is still
> possible.
>
> Does anyone plan to remove the checking?
>
> Thanks,
> Kuai
>>
>>
>>
>>
>> [1] https://lore.kernel.org/lkml/20211011073556.GA10735@lst.de/
>>
>>
>>
>> With regards,
>> Pavel Skripkin
>> .
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] nbd: fix sanity check for first_minor
2021-10-21 13:13 ` yukuai (C)
2021-10-21 13:37 ` yukuai (C)
@ 2021-10-21 16:47 ` Pavel Skripkin
1 sibling, 0 replies; 9+ messages in thread
From: Pavel Skripkin @ 2021-10-21 16:47 UTC (permalink / raw)
To: yukuai (C), josef, axboe
Cc: linux-block, nbd, linux-kernel, yi.zhang, luomeng12, Christoph Hellwig
On 10/21/21 16:13, yukuai (C) wrote:
> On 2021/10/21 20:35, Pavel Skripkin wrote:
>> On 10/21/21 15:29, Yu Kuai wrote:
>>> Yu Kuai (2):
>>> nbd: fix max value for 'first_minor'
>>> nbd: fix possible overflow for 'first_minor' in nbd_dev_add()
>>>
>>> drivers/block/nbd.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>
>> Hi, Yu!
>>
>> Thank you for the fix, but this wrong check should be just removed,
>> since root case of wrong sysfs file creation was fixed, as Christoph
>> said [1]
>
> Hi, Pavel
>
> Thanks for your response, with the root cause fixed, patch 1 is not
> needed anymore. However, the overflow case in patch 2 is still
> possible.
>
> Does anyone plan to remove the checking?
>
Hm, I thought it was already removed, but I was wrong, I guess. Let's
see what Christoph thinks about this check.
Maybe add_disk() error handling is still not in Linus tree, I haven't
checked yet. Sysfs warnings _should_ be fixed by proper error handling,
but maybe there is another problem somewhere...
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] nbd: fix sanity check for first_minor
2021-10-21 13:37 ` yukuai (C)
@ 2021-10-25 13:41 ` yukuai (C)
2021-11-01 8:06 ` yukuai (C)
0 siblings, 1 reply; 9+ messages in thread
From: yukuai (C) @ 2021-10-25 13:41 UTC (permalink / raw)
To: Pavel Skripkin, josef, axboe
Cc: linux-block, nbd, linux-kernel, yi.zhang, luomeng12, Christoph Hellwig
On 2021/10/21 21:37, yukuai (C) wrote:
> On 2021/10/21 21:13, yukuai (C) wrote:
>> On 2021/10/21 20:35, Pavel Skripkin wrote:
>>> On 10/21/21 15:29, Yu Kuai wrote:
>>>> Yu Kuai (2):
>>>> nbd: fix max value for 'first_minor'
>>>> nbd: fix possible overflow for 'first_minor' in nbd_dev_add()
>>>>
>>>> drivers/block/nbd.c | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>
>>> Hi, Yu!
>>>
>>> Thank you for the fix, but this wrong check should be just removed,
>>> since root case of wrong sysfs file creation was fixed, as Christoph
>>> said [1]
>
> Hi, Christoph
>
> By the way, if we remove the checking, there will be two kernel warnings
> when the problem happens. Maybe keeping the checking is better?
friendly ping ...
>
> [ 19.860640] sysfs: cannot create duplicate filename '/dev/block/43:0'
> [ 19.861659] CPU: 1 PID: 872 Comm: modprobe Not tainted
> 5.15.0-rc6-next-20211020-00001-g2f1
> [ 19.863175] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS ?-20190727_0738364
> [ 19.865183] Call Trace:
> [ 19.866512] <TASK>
> [ 19.866863] ? dump_stack_lvl+0x73/0x9f
> [ 19.867941] ? dump_stack+0x13/0x1b
> [ 19.868475] ? sysfs_warn_dup.cold+0x27/0x45
> [ 19.869075] ? sysfs_do_create_link_sd.isra.0+0x131/0x150
> [ 19.869818] ? sysfs_create_link+0x29/0x60
> [ 19.870459] ? device_add+0xbd6/0xf60
> [ 19.871032] ? _printk+0x5f/0x7d
> [ 19.871518] ? device_add_disk+0x153/0x5d0
> [ 19.872160] ? nbd_dev_add+0x30e/0x470 [nbd]
> [ 19.872828] ? 0xffffffffc0060000
> [ 19.873332] ? nbd_init+0x1dc/0x1000 [nbd]
> [ 19.873924] ? do_one_initcall+0x71/0x3a0
> [ 19.874534] ? gcov_event+0x70/0x690
> [ 19.875058] ? do_init_module+0xa6/0x350
> [ 19.875587] ? load_module+0x2587/0x2720
> [ 19.876130] ? kernel_read+0x31/0xb0
> [ 19.876638] ? kernel_read_file+0x15a/0x360
> [ 19.877271] ? __do_sys_finit_module+0xe5/0x190
> [ 19.877951] ? __do_sys_finit_module+0xe5/0x190
> [ 19.878563] ? __x64_sys_finit_module+0x1e/0x30
> [ 19.879182] ? do_syscall_64+0x35/0x80
> [ 19.879700] ? entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 19.880413] </TASK>
> [ 19.880806] ------------[ cut here ]------------
> [ 19.881502] WARNING: CPU: 1 PID: 872 at block/genhd.c:543
> device_add_disk+0x2af/0x5d0
> [ 19.882695] Modules linked in: nbd(+)
> [ 19.883290] CPU: 1 PID: 872 Comm: modprobe Not tainted
> 5.15.0-rc6-next-20211020-00001-g2f1
> [ 19.884823] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS ?-20190727_0738364
> [ 19.886866] RIP: 0010:device_add_disk+0x2af/0x5d0
> [ 19.887606] Code: 6e f2 f2 0b 01 49 8b 76 48 48 8b 3d db 03 f3 0b e8
> f6 ec a9 ff 48 83 050
> [ 19.890456] RSP: 0018:ffffc90000e47c70 EFLAGS: 00010202
> [ 19.891293] RAX: 0000000000000000 RBX: 0000000000000000 RCX:
> 00000000000ae001
> [ 19.892274] RDX: 00000000000ac001 RSI: ffffffff91b5906b RDI:
> 0000000000000000
> [ 19.893318] RBP: ffff8881034ad600 R08: 0000000000000000 R09:
> ffffffff915e2e69
> [ 19.894425] R10: 0000000000000014 R11: 0000000000000001 R12:
> 00000000ffffffef
> [ 19.895544] R13: 0000000000000000 R14: ffff88817d720600 R15:
> ffff88817d720648
> [ 19.896652] FS: 00007fc08c7a7040(0000) GS:ffff88882f640000(0000)
> knlGS:0000000000000000
> [ 19.897902] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 19.898768] CR2: 00007fc08bcf0395 CR3: 000000017c467000 CR4:
> 00000000000006e0
> [ 19.899754] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [ 19.900856] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400
> [ 19.901989] Call Trace:
> [ 19.902387] <TASK>
> [ 19.902732] nbd_dev_add+0x30e/0x470 [nbd]
> [ 19.903395] ? 0xffffffffc0060000
> [ 19.903917] nbd_init+0x1dc/0x1000 [nbd]
> [ 19.904536] do_one_initcall+0x71/0x3a0
> [ 19.905166] ? gcov_event+0x70/0x690
> [ 19.905745] do_init_module+0xa6/0x350
> [ 19.906351] load_module+0x2587/0x2720
> [ 19.906932] ? kernel_read+0x31/0xb0
> [ 19.907509] ? kernel_read_file+0x15a/0x360
> [ 19.908195] ? __do_sys_finit_module+0xe5/0x190
> [ 19.908894] __do_sys_finit_module+0xe5/0x190
> [ 19.909591] __x64_sys_finit_module+0x1e/0x30
> [ 19.910289] do_syscall_64+0x35/0x80
> [ 19.910855] entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 19.911652] RIP: 0033:0x7fc08bc7a4e9
> [ 19.912231] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00
> 48 89 f8 48 89 f7 488
> [ 19.915026] RSP: 002b:00007fff0ddf6fd8 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000139
> [ 19.916080] RAX: ffffffffffffffda RBX: 00005596400853e0 RCX:
> 00007fc08bc7a4e9
> [ 19.917180] RDX: 0000000000000000 RSI: 000055963fe1bc26 RDI:
> 0000000000000003
> [ 19.918302] RBP: 000055963fe1bc26 R08: 0000000000000000 R09:
> 0000000000000000
> [ 19.919412] R10: 0000000000000003 R11: 0000000000000246 R12:
> 0000000000000000
> [ 19.920524] R13: 00005596400854f0 R14: 0000000000040000 R15:
> 00005596400853e0
> [ 19.921629] </TASK>
> [ 19.922005] ---[ end trace e09ecf130812479d ]---
>
> Thanks,
> Kuai
>>
>> Hi, Pavel
>>
>> Thanks for your response, with the root cause fixed, patch 1 is not
>> needed anymore. However, the overflow case in patch 2 is still
>> possible.
>>
>> Does anyone plan to remove the checking?
>>
>> Thanks,
>> Kuai
>>>
>>>
>>>
>>>
>>> [1] https://lore.kernel.org/lkml/20211011073556.GA10735@lst.de/
>>>
>>>
>>>
>>> With regards,
>>> Pavel Skripkin
>>> .
>>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] nbd: fix sanity check for first_minor
2021-10-25 13:41 ` yukuai (C)
@ 2021-11-01 8:06 ` yukuai (C)
0 siblings, 0 replies; 9+ messages in thread
From: yukuai (C) @ 2021-11-01 8:06 UTC (permalink / raw)
To: Pavel Skripkin, josef, axboe
Cc: linux-block, nbd, linux-kernel, yi.zhang, luomeng12, Christoph Hellwig
On 2021/10/25 21:41, yukuai (C) wrote:
> On 2021/10/21 21:37, yukuai (C) wrote:
>> On 2021/10/21 21:13, yukuai (C) wrote:
>>> On 2021/10/21 20:35, Pavel Skripkin wrote:
>>>> On 10/21/21 15:29, Yu Kuai wrote:
>>>>> Yu Kuai (2):
>>>>> nbd: fix max value for 'first_minor'
>>>>> nbd: fix possible overflow for 'first_minor' in nbd_dev_add()
>>>>>
>>>>> drivers/block/nbd.c | 6 +++---
>>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>
>>>> Hi, Yu!
>>>>
>>>> Thank you for the fix, but this wrong check should be just removed,
>>>> since root case of wrong sysfs file creation was fixed, as Christoph
>>>> said [1]
>>
>> Hi, Christoph
>>
>> By the way, if we remove the checking, there will be two kernel warnings
>> when the problem happens. Maybe keeping the checking is better?
>
> friendly ping ...
friendly ping ...
>>
>> [ 19.860640] sysfs: cannot create duplicate filename '/dev/block/43:0'
>> [ 19.861659] CPU: 1 PID: 872 Comm: modprobe Not tainted
>> 5.15.0-rc6-next-20211020-00001-g2f1
>> [ 19.863175] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
>> BIOS ?-20190727_0738364
>> [ 19.865183] Call Trace:
>> [ 19.866512] <TASK>
>> [ 19.866863] ? dump_stack_lvl+0x73/0x9f
>> [ 19.867941] ? dump_stack+0x13/0x1b
>> [ 19.868475] ? sysfs_warn_dup.cold+0x27/0x45
>> [ 19.869075] ? sysfs_do_create_link_sd.isra.0+0x131/0x150
>> [ 19.869818] ? sysfs_create_link+0x29/0x60
>> [ 19.870459] ? device_add+0xbd6/0xf60
>> [ 19.871032] ? _printk+0x5f/0x7d
>> [ 19.871518] ? device_add_disk+0x153/0x5d0
>> [ 19.872160] ? nbd_dev_add+0x30e/0x470 [nbd]
>> [ 19.872828] ? 0xffffffffc0060000
>> [ 19.873332] ? nbd_init+0x1dc/0x1000 [nbd]
>> [ 19.873924] ? do_one_initcall+0x71/0x3a0
>> [ 19.874534] ? gcov_event+0x70/0x690
>> [ 19.875058] ? do_init_module+0xa6/0x350
>> [ 19.875587] ? load_module+0x2587/0x2720
>> [ 19.876130] ? kernel_read+0x31/0xb0
>> [ 19.876638] ? kernel_read_file+0x15a/0x360
>> [ 19.877271] ? __do_sys_finit_module+0xe5/0x190
>> [ 19.877951] ? __do_sys_finit_module+0xe5/0x190
>> [ 19.878563] ? __x64_sys_finit_module+0x1e/0x30
>> [ 19.879182] ? do_syscall_64+0x35/0x80
>> [ 19.879700] ? entry_SYSCALL_64_after_hwframe+0x44/0xae
>> [ 19.880413] </TASK>
>> [ 19.880806] ------------[ cut here ]------------
>> [ 19.881502] WARNING: CPU: 1 PID: 872 at block/genhd.c:543
>> device_add_disk+0x2af/0x5d0
>> [ 19.882695] Modules linked in: nbd(+)
>> [ 19.883290] CPU: 1 PID: 872 Comm: modprobe Not tainted
>> 5.15.0-rc6-next-20211020-00001-g2f1
>> [ 19.884823] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
>> BIOS ?-20190727_0738364
>> [ 19.886866] RIP: 0010:device_add_disk+0x2af/0x5d0
>> [ 19.887606] Code: 6e f2 f2 0b 01 49 8b 76 48 48 8b 3d db 03 f3 0b
>> e8 f6 ec a9 ff 48 83 050
>> [ 19.890456] RSP: 0018:ffffc90000e47c70 EFLAGS: 00010202
>> [ 19.891293] RAX: 0000000000000000 RBX: 0000000000000000 RCX:
>> 00000000000ae001
>> [ 19.892274] RDX: 00000000000ac001 RSI: ffffffff91b5906b RDI:
>> 0000000000000000
>> [ 19.893318] RBP: ffff8881034ad600 R08: 0000000000000000 R09:
>> ffffffff915e2e69
>> [ 19.894425] R10: 0000000000000014 R11: 0000000000000001 R12:
>> 00000000ffffffef
>> [ 19.895544] R13: 0000000000000000 R14: ffff88817d720600 R15:
>> ffff88817d720648
>> [ 19.896652] FS: 00007fc08c7a7040(0000) GS:ffff88882f640000(0000)
>> knlGS:0000000000000000
>> [ 19.897902] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 19.898768] CR2: 00007fc08bcf0395 CR3: 000000017c467000 CR4:
>> 00000000000006e0
>> [ 19.899754] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
>> 0000000000000000
>> [ 19.900856] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
>> 0000000000000400
>> [ 19.901989] Call Trace:
>> [ 19.902387] <TASK>
>> [ 19.902732] nbd_dev_add+0x30e/0x470 [nbd]
>> [ 19.903395] ? 0xffffffffc0060000
>> [ 19.903917] nbd_init+0x1dc/0x1000 [nbd]
>> [ 19.904536] do_one_initcall+0x71/0x3a0
>> [ 19.905166] ? gcov_event+0x70/0x690
>> [ 19.905745] do_init_module+0xa6/0x350
>> [ 19.906351] load_module+0x2587/0x2720
>> [ 19.906932] ? kernel_read+0x31/0xb0
>> [ 19.907509] ? kernel_read_file+0x15a/0x360
>> [ 19.908195] ? __do_sys_finit_module+0xe5/0x190
>> [ 19.908894] __do_sys_finit_module+0xe5/0x190
>> [ 19.909591] __x64_sys_finit_module+0x1e/0x30
>> [ 19.910289] do_syscall_64+0x35/0x80
>> [ 19.910855] entry_SYSCALL_64_after_hwframe+0x44/0xae
>> [ 19.911652] RIP: 0033:0x7fc08bc7a4e9
>> [ 19.912231] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40
>> 00 48 89 f8 48 89 f7 488
>> [ 19.915026] RSP: 002b:00007fff0ddf6fd8 EFLAGS: 00000246 ORIG_RAX:
>> 0000000000000139
>> [ 19.916080] RAX: ffffffffffffffda RBX: 00005596400853e0 RCX:
>> 00007fc08bc7a4e9
>> [ 19.917180] RDX: 0000000000000000 RSI: 000055963fe1bc26 RDI:
>> 0000000000000003
>> [ 19.918302] RBP: 000055963fe1bc26 R08: 0000000000000000 R09:
>> 0000000000000000
>> [ 19.919412] R10: 0000000000000003 R11: 0000000000000246 R12:
>> 0000000000000000
>> [ 19.920524] R13: 00005596400854f0 R14: 0000000000040000 R15:
>> 00005596400853e0
>> [ 19.921629] </TASK>
>> [ 19.922005] ---[ end trace e09ecf130812479d ]---
>>
>> Thanks,
>> Kuai
>>>
>>> Hi, Pavel
>>>
>>> Thanks for your response, with the root cause fixed, patch 1 is not
>>> needed anymore. However, the overflow case in patch 2 is still
>>> possible.
>>>
>>> Does anyone plan to remove the checking?
>>>
>>> Thanks,
>>> Kuai
>>>>
>>>>
>>>>
>>>>
>>>> [1] https://lore.kernel.org/lkml/20211011073556.GA10735@lst.de/
>>>>
>>>>
>>>>
>>>> With regards,
>>>> Pavel Skripkin
>>>> .
>>>>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-11-01 8:06 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21 12:29 [PATCH 0/2] nbd: fix sanity check for first_minor Yu Kuai
2021-10-21 12:29 ` [PATCH 1/2] nbd: fix max value for 'first_minor' Yu Kuai
2021-10-21 12:29 ` [PATCH 2/2] nbd: fix possible overflow for 'first_minor' in nbd_dev_add() Yu Kuai
2021-10-21 12:35 ` [PATCH 0/2] nbd: fix sanity check for first_minor Pavel Skripkin
2021-10-21 13:13 ` yukuai (C)
2021-10-21 13:37 ` yukuai (C)
2021-10-25 13:41 ` yukuai (C)
2021-11-01 8:06 ` yukuai (C)
2021-10-21 16:47 ` Pavel Skripkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).