linux-kernel - Re: [PATCH] net/9p: use a dedicated spinlock for trans

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9d4c7f38-6a30-3efb-7887-9c6224494ffd@I-love.SAKURA.ne.jp>
Date:   Sun, 4 Sep 2022 22:03:49 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:     Dominique Martinet <asmadeus@...ewreck.org>,
        v9fs-developer@...ts.sourceforge.net
Cc:     syzkaller-bugs@...glegroups.com, linux_oss@...debyte.com,
        linux-kernel@...r.kernel.org,
        syzbot <syzbot+2f20b523930c32c160cc@...kaller.appspotmail.com>
Subject: Re: [PATCH] net/9p: use a dedicated spinlock for trans_fd

On 2022/09/04 20:29, Dominique Martinet wrote:
> Since the locks actually protect different things in client.c and in
> trans_fd.c, just replace trans_fd.c's lock by a new one specific to the
> transport instead of using spin_lock_irq* variants everywhere
> (client.c's protect the idr for tag allocations, while
> trans_fd.c's protects its own req list and request status field
> that acts as the transport's state machine)

OK, I figured out what this patch changes.

  $ grep -nF -- '->lock' *.[ch]
  client.c:286:   spin_lock_irq(&c->lock);
  client.c:293:   spin_unlock_irq(&c->lock);
  client.c:367:   spin_lock_irqsave(&c->lock, flags);
  client.c:369:   spin_unlock_irqrestore(&c->lock, flags);
  client.c:816:   spin_lock_irq(&clnt->lock);
  client.c:819:   spin_unlock_irq(&clnt->lock);
  client.c:838:   spin_lock_irqsave(&clnt->lock, flags);
  client.c:840:   spin_unlock_irqrestore(&clnt->lock, flags);
  client.c:945:   spin_lock_init(&clnt->lock);
  trans_virtio.c:139:     spin_lock_irqsave(&chan->lock, flags);
  trans_virtio.c:151:     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:268:     spin_lock_irqsave(&chan->lock, flags);
  trans_virtio.c:287:                     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:296:                     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:303:     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:474:     spin_lock_irqsave(&chan->lock, flags);
  trans_virtio.c:515:                     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:524:                     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:532:     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:621:     spin_lock_init(&chan->lock);
  trans_xen.c:142:        spin_lock_irqsave(&ring->lock, flags);
  trans_xen.c:149:                spin_unlock_irqrestore(&ring->lock, flags);
  trans_xen.c:164:        spin_unlock_irqrestore(&ring->lock, flags);
  trans_xen.c:314:        spin_lock_init(&ring->lock);

This patch changes "struct p9_client"->lock to be used for only
protecting idr, as explained at

 * @lock: protect @fids and @reqs

line. Makes sense and looks correct.

> Tetsuo Handa-san, thank you very much!
> I'm sorry for not respecting your opinion but it's been a pleasure to
> have submissions from someone on JST :)

Thank you for responding quickly. :-)