scsi: sg: fix refcount warning

Commit Message

Keith Busch April 4, 2024, 11:18 p.m. UTC

From: Keith Busch <[e-mail]>

The refcount is taken multiple times on the open, from sg_add_sfp() and
sg_get_dev(). Both are still counted at the point of the warning, so it looks
like the WARN just wants to ensure at least one live reference exists rather
than exactly one reference.

This fixes the warning:

 ------------[ cut here ]------------
 WARNING: CPU: 8 PID: 394 at drivers/scsi/sg.c:2236 sg_remove_sfp_usercontext+0x1ad/0x1d0 [sg]
 Modules linked in: sg(E) nfit(E) libnvdimm(E) kvm_intel(E) kvm(E) evdev(E) serio_raw(E) e1000(E) i2c_piix4(E) button(E) sch_fq_codel(E) drm(E) fuse(E) drm_panel_orientation_quirks(E) autofs4(E)
 CPU: 8 PID: 394 Comm: kworker/8:3 Tainted: G        W   E      6.9.0-rc2+ #545
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.2-14-g1e1da7a96300-prebuilt.qemu.org 04/01/2014
 Workqueue: events sg_remove_sfp_usercontext [sg]
 RIP: 0010:sg_remove_sfp_usercontext+0x1ad/0x1d0 [sg]
 Code: e8 18 9d 49 e1 eb a2 4c 8b 04 24 49 8d 56 6d 48 c7 c1 d8 84 37 a0 48 c7 c7 fb 81 37 a0 49 8b 36 e8 28 14 74 e1 e9 48 ff ff ff <0f> 0b 41 8b b6 98 00 00 00 48 c7 c7 03 85 37 a0 e8 3e 02 e1 e0 e9
 RSP: 0018:ffffc90000687e30 EFLAGS: 00010202
 RAX: 0000000000000002 RBX: ffff88801ecd9328 RCX: 0000000000000041
 RDX: 0000000000000040 RSI: 000000000003a1c0 RDI: ffff88801ecd8000
 RBP: ffff88801ecd8080 R08: 0000000000000000 R09: ffff88803ea3b340
 R10: 0000000000000214 R11: 0000000000000000 R12: ffff888004969000
 R13: ffff88803ea34240 R14: ffff888015ec1980 R15: ffff888015ec0900
 FS:  0000000000000000(0000) GS:ffff88803ea00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00007f9ab9cc8c60 CR3: 000000001aa7c003 CR4: 0000000000770ef0
 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
 PKRU: 55555554
 Call Trace:
  <TASK>
  ? __warn+0x7f/0x120
  ? sg_remove_sfp_usercontext+0x1ad/0x1d0 [sg]
  ? report_bug+0x17d/0x190
  ? handle_bug+0x42/0x70
  ? exc_invalid_op+0x14/0x70
  ? asm_exc_invalid_op+0x16/0x20
  ? sg_remove_sfp_usercontext+0x1ad/0x1d0 [sg]
  process_one_work+0x13e/0x300
  worker_thread+0x2cb/0x3e0
  ? __pfx_worker_thread+0x10/0x10
  kthread+0xc4/0xf0
  ? __pfx_kthread+0x10/0x10
  ret_from_fork+0x2d/0x50
  ? __pfx_kthread+0x10/0x10
  ret_from_fork_asm+0x1a/0x30
  </TASK>
 ---[ end trace 0000000000000000 ]---

Created from this simple reproducer:

---

int main() {
    int fd = open("/dev/sg0", O_RDWR);

    if (fd < 0)
        return 1;

    close(fd);
    return 0;
}
--

Fixes: 27f58c04a8f438 ("scsi: sg: Avoid sg device teardown race")
Cc: Alexander Wetzel <[e-mail]>
Reported-by: Jakub Kicinski <[e-mail]>
Signed-off-by: Keith Busch <[e-mail]>
---
 drivers/scsi/sg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Keith Busch April 4, 2024, 11:32 p.m. UTC | #1

On Thu, Apr 04, 2024 at 04:18:40PM -0700, Keith Busch wrote:
> This fixes the warning:
> 
>  ------------[ cut here ]------------
>  WARNING: CPU: 8 PID: 394 at drivers/scsi/sg.c:2236 sg_remove_sfp_usercontext+0x1ad/0x1d0 [sg]

I see the tree we're testing is a little behind the latest scsi, and a
proper fix was already committed:

  https://lore.kernel.org/linux-scsi/[e-mail]/T/#t

Patch

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 386981c6976a5..eba8fc8c88a51 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -2233,7 +2233,7 @@  sg_remove_sfp_usercontext(struct work_struct *work)
 			"sg_remove_sfp: sfp=0x%p\n", sfp));
 	kfree(sfp);
 
-	WARN_ON_ONCE(kref_read(&sdp->d_ref) != 1);
+	WARN_ON_ONCE(kref_read(&sdp->d_ref) < 1);
 	kref_put(&sdp->d_ref, sg_device_destroy);
 	scsi_device_put(device);
 	module_put(THIS_MODULE);