[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89iKxjOPyP7h-8bCtx1SwCM1FaXDAXfcdCW7uXxKsy49L3w@mail.gmail.com>
Date: Fri, 24 Oct 2025 04:49:55 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Lizhi Xu <lizhi.xu@...driver.com>
Cc: davem@...emloft.net, horms@...nel.org, jreuter@...na.de, kuba@...nel.org,
kuniyu@...gle.com, linux-hams@...r.kernel.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, pabeni@...hat.com,
syzbot+caa052a0958a9146870d@...kaller.appspotmail.com,
syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH V3] net: rose: Prevent the use of freed digipeat
On Fri, Oct 24, 2025 at 2:39 AM Lizhi Xu <lizhi.xu@...driver.com> wrote:
>
> There is no synchronization between the two timers, rose_t0timer_expiry
> and rose_timer_expiry.
> rose_timer_expiry() puts the neighbor when the rose state is ROSE_STATE_2.
> However, rose_t0timer_expiry() does initiate a restart request on the
> neighbor.
> When rose_t0timer_expiry() accesses the released neighbor member digipeat,
> a UAF is triggered.
>
> To avoid this UAF, defer the put operation to rose_t0timer_expiry() and
> stop restarting t0timer after putting the neighbor.
>
> When putting the neighbor, set the neighbor to NULL. Setting neighbor to
> NULL prevents rose_t0timer_expiry() from restarting t0timer.
>
> syzbot reported a slab-use-after-free Read in ax25_find_cb.
> BUG: KASAN: slab-use-after-free in ax25_find_cb+0x3b8/0x3f0 net/ax25/af_ax25.c:237
> Read of size 1 at addr ffff888059c704c0 by task syz.6.2733/17200
> Call Trace:
> ax25_find_cb+0x3b8/0x3f0 net/ax25/af_ax25.c:237
> ax25_send_frame+0x157/0xb60 net/ax25/ax25_out.c:55
> rose_send_frame+0xcc/0x2c0 net/rose/rose_link.c:106
> rose_transmit_restart_request+0x1b8/0x240 net/rose/rose_link.c:198
> rose_t0timer_expiry+0x1d/0x150 net/rose/rose_link.c:83
>
> Freed by task 17183:
> kfree+0x2b8/0x6d0 mm/slub.c:6826
> rose_neigh_put include/net/rose.h:165 [inline]
> rose_timer_expiry+0x537/0x630 net/rose/rose_timer.c:183
>
> Fixes: d860d1faa6b2 ("net: rose: convert 'use' field to refcount_t")
> Reported-by: syzbot+caa052a0958a9146870d@...kaller.appspotmail.com
> Signed-off-by: Lizhi Xu <lizhi.xu@...driver.com>
> ---
> V1 -> V2: Putting the neighbor stops t0timer from automatically starting
> V2 -> V3: add rose_neigh_putex for set rose neigh to NULL
>
> include/net/rose.h | 12 ++++++++++++
> net/rose/rose_link.c | 5 +++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/include/net/rose.h b/include/net/rose.h
> index 2b5491bbf39a..33de310ba778 100644
> --- a/include/net/rose.h
> +++ b/include/net/rose.h
> @@ -167,6 +167,18 @@ static inline void rose_neigh_put(struct rose_neigh *rose_neigh)
> }
> }
>
> +static inline void rose_neigh_putex(struct rose_neigh **roseneigh)
> +{
> + struct rose_neigh *rose_neigh = *roseneigh;
> + if (refcount_dec_and_test(&rose_neigh->use)) {
> + if (rose_neigh->ax25)
> + ax25_cb_put(rose_neigh->ax25);
> + kfree(rose_neigh->digipeat);
> + kfree(rose_neigh);
> + *roseneigh = NULL;
> + }
> +}
You have not even compiled this patch.
Also please carefully read Documentation/process/maintainer-netdev.rst
Resending after review
~~~~~~~~~~~~~~~~~~~~~~
Allow at least 24 hours to pass between postings. This will ensure reviewers
from all geographical locations have a chance to chime in. Do not wait
too long (weeks) between postings either as it will make it harder for reviewers
to recall all the context.
Make sure you address all the feedback in your new posting. Do not post a new
version of the code if the discussion about the previous version is still
ongoing, unless directly instructed by a reviewer.
The new version of patches should be posted as a separate thread,
not as a reply to the previous posting. Change log should include a link
to the previous posting (see :ref:`Changes requested`).
Powered by blists - more mailing lists