[PATCH v2] aoe: fix the potential use-after-free problem in more places

[PATCH v2] aoe: fix the potential use-after-free problem in more places

[Chun-Yi Lee]

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(-)

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;
 }
-- 
2.35.3

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[Greg KH]

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>

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[Markus Elfring]

>                   … 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

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[Markus Elfring]

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

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[joeyli]

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 

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[joeyli]

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

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[joeyli]

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

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[Markus Elfring]

>> 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

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[Markus Elfring]

>>>                   … 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

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[joeyli]

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 

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[joeyli]

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 

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[Greg KH]

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

Re: [PATCH v2] aoe: fix the potential use-after-free problem in more places

[kernel test robot]

Hi,

Thanks for your patch.

FYI: kernel test robot notices the stable kernel rule is not satisfied.

The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1

Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree.
Subject: [PATCH v2] aoe: fix the potential use-after-free problem in more places
Link: https://lore.kernel.org/stable/20240624064418.27043-1-jlee%40suse.com

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki