| Message ID | 20240624064418.27043-1-jlee@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] aoe: fix the potential use-after-free problem in more places | expand |
On Mon, Jun 24, 2024 at 02:44:18PM +0800, Chun-Yi Lee wrote: > For fixing CVE-2023-6270, f98364e92662 ("aoe: fix the potential > use-after-free problem in aoecmd_cfg_pkts") makes tx() calling dev_put() > instead of doing in aoecmd_cfg_pkts(). It avoids that the tx() runs > into use-after-free. > > Then Nicolai Stange found more places in aoe have potential use-after-free > problem with tx(). e.g. revalidate(), aoecmd_ata_rw(), resend(), probe() > and aoecmd_cfg_rsp(). Those functions also use aoenet_xmit() to push > packet to tx queue. So they should also use dev_hold() to increase the > refcnt of skb->dev. > > Link: https://nvd.nist.gov/vuln/detail/CVE-2023-6270 > Fixes: f98364e92662 ("aoe: fix the potential use-after-free problem in aoecmd_cfg_pkts") > Reported-by: Nicolai Stange <nstange@suse.com> > Signed-off-by: Chun-Yi Lee <jlee@suse.com> > --- <formletter> This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. </formletter>
> … So they should also use dev_hold() to increase the > refcnt of skb->dev. … reference counter of “skb->dev”? … > Fixes: f98364e92662 ("aoe: fix the potential use-after-free problem in aoecmd_cfg_pkts") Would you like to add a “stable tag”? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v6.10-rc4#n34 Will an adjusted summary phrase become more helpful? Regards, Markus
Please reconsider the version identification in this patch subject once more. … > --- > > v2: > - Improve patch description … How many patch variations were discussed and reviewed in the meantime? Regards, Markus
Hi Greg, On Mon, Jun 24, 2024 at 09:05:59AM +0200, Greg KH wrote: > On Mon, Jun 24, 2024 at 02:44:18PM +0800, Chun-Yi Lee wrote: > > For fixing CVE-2023-6270, f98364e92662 ("aoe: fix the potential > > use-after-free problem in aoecmd_cfg_pkts") makes tx() calling dev_put() > > instead of doing in aoecmd_cfg_pkts(). It avoids that the tx() runs > > into use-after-free. > > > > Then Nicolai Stange found more places in aoe have potential use-after-free > > problem with tx(). e.g. revalidate(), aoecmd_ata_rw(), resend(), probe() > > and aoecmd_cfg_rsp(). Those functions also use aoenet_xmit() to push > > packet to tx queue. So they should also use dev_hold() to increase the > > refcnt of skb->dev. > > > > Link: https://nvd.nist.gov/vuln/detail/CVE-2023-6270 > > Fixes: f98364e92662 ("aoe: fix the potential use-after-free problem in aoecmd_cfg_pkts") > > Reported-by: Nicolai Stange <nstange@suse.com> > > Signed-off-by: Chun-Yi Lee <jlee@suse.com> > > --- > > <formletter> > > This is not the correct way to submit patches for inclusion in the > stable kernel tree. Please read: > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > for how to do this properly. > > </formletter> Thanks for your reminder. I will remove stable@vger.kernel.org in next version. Joey Lee
Hi Markus, On Mon, Jun 24, 2024 at 10:40:13AM +0200, Markus Elfring wrote: > > … So they should also use dev_hold() to increase the > > refcnt of skb->dev. > … > > reference counter of “skb->dev”? > Yes, I will update my wording. Thanks! Joey Lee > > … > > Fixes: f98364e92662 ("aoe: fix the potential use-after-free problem in aoecmd_cfg_pkts") > > Would you like to add a “stable tag”? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v6.10-rc4#n34 > > > Will an adjusted summary phrase become more helpful? > > Regards, > Markus
On Mon, Jun 24, 2024 at 11:27:53AM +0200, Markus Elfring wrote: > Please reconsider the version identification in this patch subject once more. > > > … > > --- > > > > v2: > > - Improve patch description > … > > How many patch variations were discussed and reviewed in the meantime? > Only v2. I sent v2 patch again because nobody response my code in patch. But I still want to grap comments for my code. Thanks Joey Lee
>> Please reconsider the version identification in this patch subject once more. >> >> >> … >>> --- >>> >>> v2: >>> - Improve patch description >> … >> >> How many patch variations were discussed and reviewed in the meantime? >> > > Only v2. I sent v2 patch again because nobody response my code in patch. > But I still want to grap comments for my code. How does such a feedback fit to my previous patch review? https://lore.kernel.org/r/e8331545-d261-44af-b500-93b90d77d8b7@web.de/ https://lkml.org/lkml/2024/5/14/551 Regards, Markus
>>> … So they should also use dev_hold() to increase the >>> refcnt of skb->dev. >> … >> >> reference counter of “skb->dev”? > > Yes, I will update my wording. Would you like to improve such a change description also with imperative wordings? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc4#n94 How do you think about the text “Prevent use-after-free issues at more places” for a summary phrase? Regards, Markus
On Mon, Jun 24, 2024 at 01:28:54PM +0200, Markus Elfring wrote: > >> Please reconsider the version identification in this patch subject once more. > >> > >> > >> … > >>> --- > >>> > >>> v2: > >>> - Improve patch description > >> … > >> > >> How many patch variations were discussed and reviewed in the meantime? > >> > > > > Only v2. I sent v2 patch again because nobody response my code in patch. > > But I still want to grap comments for my code. > > How does such a feedback fit to my previous patch review? > https://lore.kernel.org/r/e8331545-d261-44af-b500-93b90d77d8b7@web.de/ > https://lkml.org/lkml/2024/5/14/551 > I want to collect comment of my code in patch then send next version. Joey Lee
On Mon, Jun 24, 2024 at 01:43:25PM +0200, Markus Elfring wrote: > >>> … So they should also use dev_hold() to increase the > >>> refcnt of skb->dev. > >> … > >> > >> reference counter of “skb->dev”? > > > > Yes, I will update my wording. > > Would you like to improve such a change description also with imperative wordings? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc4#n94 > > > How do you think about the text “Prevent use-after-free issues at more places” > for a summary phrase? > Thanks for your suggestion. I will update the wording in next version. Joey Lee
On Mon, Jun 24, 2024 at 07:54:45PM +0800, joeyli wrote: > On Mon, Jun 24, 2024 at 01:43:25PM +0200, Markus Elfring wrote: > > >>> … So they should also use dev_hold() to increase the > > >>> refcnt of skb->dev. > > >> … > > >> > > >> reference counter of “skb->dev”? > > > > > > Yes, I will update my wording. > > > > Would you like to improve such a change description also with imperative wordings? > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc4#n94 > > > > > > How do you think about the text “Prevent use-after-free issues at more places” > > for a summary phrase? > > > > Thanks for your suggestion. I will update the wording in next version. Hi, This is the semi-friendly patch-bot of Greg Kroah-Hartman. Markus, you seem to have sent a nonsensical or otherwise pointless review comment to a patch submission on a Linux kernel developer mailing list. I strongly suggest that you not do this anymore. Please do not bother developers who are actively working to produce patches and features with comments that, in the end, are a waste of time. Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them. thanks, greg k-h's patch email bot
diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c index cc9077b588d7..d1f4ddc57645 100644 --- a/drivers/block/aoe/aoecmd.c +++ b/drivers/block/aoe/aoecmd.c @@ -361,6 +361,7 @@ ata_rw_frameinit(struct frame *f) } ah->cmdstat = ATA_CMD_PIO_READ | writebit | extbit; + dev_hold(t->ifp->nd); skb->dev = t->ifp->nd; } @@ -401,6 +402,8 @@ aoecmd_ata_rw(struct aoedev *d) __skb_queue_head_init(&queue); __skb_queue_tail(&queue, skb); aoenet_xmit(&queue); + } else { + dev_put(f->t->ifp->nd); } return 1; } @@ -483,10 +486,13 @@ resend(struct aoedev *d, struct frame *f) memcpy(h->dst, t->addr, sizeof h->dst); memcpy(h->src, t->ifp->nd->dev_addr, sizeof h->src); + dev_hold(t->ifp->nd); skb->dev = t->ifp->nd; skb = skb_clone(skb, GFP_ATOMIC); - if (skb == NULL) + if (skb == NULL) { + dev_put(t->ifp->nd); return; + } f->sent = ktime_get(); __skb_queue_head_init(&queue); __skb_queue_tail(&queue, skb); @@ -617,6 +623,8 @@ probe(struct aoetgt *t) __skb_queue_head_init(&queue); __skb_queue_tail(&queue, skb); aoenet_xmit(&queue); + } else { + dev_put(f->t->ifp->nd); } } @@ -1395,6 +1403,7 @@ aoecmd_ata_id(struct aoedev *d) ah->cmdstat = ATA_CMD_ID_ATA; ah->lba3 = 0xa0; + dev_hold(t->ifp->nd); skb->dev = t->ifp->nd; d->rttavg = RTTAVG_INIT; @@ -1404,6 +1413,8 @@ aoecmd_ata_id(struct aoedev *d) skb = skb_clone(skb, GFP_ATOMIC); if (skb) f->sent = ktime_get(); + else + dev_put(t->ifp->nd); return skb; }
For fixing CVE-2023-6270, f98364e92662 ("aoe: fix the potential use-after-free problem in aoecmd_cfg_pkts") makes tx() calling dev_put() instead of doing in aoecmd_cfg_pkts(). It avoids that the tx() runs into use-after-free. Then Nicolai Stange found more places in aoe have potential use-after-free problem with tx(). e.g. revalidate(), aoecmd_ata_rw(), resend(), probe() and aoecmd_cfg_rsp(). Those functions also use aoenet_xmit() to push packet to tx queue. So they should also use dev_hold() to increase the refcnt of skb->dev. Link: https://nvd.nist.gov/vuln/detail/CVE-2023-6270 Fixes: f98364e92662 ("aoe: fix the potential use-after-free problem in aoecmd_cfg_pkts") Reported-by: Nicolai Stange <nstange@suse.com> Signed-off-by: Chun-Yi Lee <jlee@suse.com> --- v2: - Improve patch description - Improved wording - Add oneline summary of the commit f98364e92662 - Used curly brackets in the if-else blocks. drivers/block/aoe/aoecmd.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)