drivers/net/caif/caif_serial.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
Add NULL pointer checks for ser and ser->dev in ldisc_receive() to
prevent memory leaks when the function is called during device close
or in race conditions where tty->disc_data or ser->dev may be NULL.
The memory leak occurred because ser->dev was accessed before checking
if ser or ser->dev was NULL, which could cause a NULL pointer
dereference or use of freed memory. Additionally, set tty->disc_data
to NULL in ldisc_close() to prevent receive_buf() from using a freed
ser pointer after the line discipline is closed.
Reported-by: syzbot+f9d847b2b84164fa69f3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f9d847b2b84164fa69f3
Fixes: 9b27105b4a44 ("net-caif-driver: add CAIF serial driver (ldisc)")
CC: stable@vger.kernel.org
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
v2:
1.Combine NULL pointer checks for ser and ser->dev in ldisc_receive()
2.Set tty->disc_data = NULL in ldisc_close() to prevent receive_buf()
from using a freed ser pointer after close.
3.Add NULL pointer check for ser in ldisc_close()
---
drivers/net/caif/caif_serial.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index c398ac42eae9..970237a3ccca 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -152,6 +152,8 @@ static void ldisc_receive(struct tty_struct *tty, const u8 *data,
int ret;
ser = tty->disc_data;
+ if (!ser || !ser->dev)
+ return;
/*
* NOTE: flags may contain information about break or overrun.
@@ -170,8 +172,6 @@ static void ldisc_receive(struct tty_struct *tty, const u8 *data,
return;
}
- BUG_ON(ser->dev == NULL);
-
/* Get a suitable caif packet and copy in data. */
skb = netdev_alloc_skb(ser->dev, count+1);
if (skb == NULL)
@@ -355,11 +355,15 @@ static void ldisc_close(struct tty_struct *tty)
{
struct ser_device *ser = tty->disc_data;
+ if (!ser)
+ return;
+
tty_kref_put(ser->tty);
spin_lock(&ser_lock);
list_move(&ser->node, &ser_release_list);
spin_unlock(&ser_lock);
+ tty->disc_data = NULL;
schedule_work(&ser_release_work);
}
--
2.43.0On 18. 01. 26, 18:44, Osama Abdelkader wrote:
> Add NULL pointer checks for ser and ser->dev in ldisc_receive() to
> prevent memory leaks when the function is called during device close
> or in race conditions where tty->disc_data or ser->dev may be NULL.
>
> The memory leak occurred because ser->dev was accessed before checking
> if ser or ser->dev was NULL, which could cause a NULL pointer
> dereference or use of freed memory. Additionally, set tty->disc_data
> to NULL in ldisc_close() to prevent receive_buf() from using a freed
> ser pointer after the line discipline is closed.
>
> Reported-by: syzbot+f9d847b2b84164fa69f3@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f9d847b2b84164fa69f3
> Fixes: 9b27105b4a44 ("net-caif-driver: add CAIF serial driver (ldisc)")
> CC: stable@vger.kernel.org
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
> v2:
> 1.Combine NULL pointer checks for ser and ser->dev in ldisc_receive()
> 2.Set tty->disc_data = NULL in ldisc_close() to prevent receive_buf()
> from using a freed ser pointer after close.
> 3.Add NULL pointer check for ser in ldisc_close()
> ---
> drivers/net/caif/caif_serial.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
> index c398ac42eae9..970237a3ccca 100644
> --- a/drivers/net/caif/caif_serial.c
> +++ b/drivers/net/caif/caif_serial.c
> @@ -152,6 +152,8 @@ static void ldisc_receive(struct tty_struct *tty, const u8 *data,
> int ret;
>
> ser = tty->disc_data;
> + if (!ser || !ser->dev)
> + return;
>
> /*
> * NOTE: flags may contain information about break or overrun.
> @@ -170,8 +172,6 @@ static void ldisc_receive(struct tty_struct *tty, const u8 *data,
> return;
> }
>
> - BUG_ON(ser->dev == NULL);
> -
> /* Get a suitable caif packet and copy in data. */
> skb = netdev_alloc_skb(ser->dev, count+1);
Wait, the reported error is memory leak of mem allocated here. So both
ser and ser->dev appear to be valid?
So instead, does netif_rx() return an error few lines below for some
reason? So should skb just be freed in that path?
if (skb == NULL)
return;
skb_put_data(skb, data, count);
skb->protocol = htons(ETH_P_CAIF);
skb_reset_mac_header(skb);
debugfs_rx(ser, data, count);
/* Push received packet up the stack. */
ret = netif_rx(skb);
if (!ret) {
ser->dev->stats.rx_packets++;
ser->dev->stats.rx_bytes += count;
} else
++ser->dev->stats.rx_dropped;
Not calling skb_free() in this else path _is_ a BUG™ in any case IMO.
thanks,
--
js
suse labs
On 19. 01. 26, 7:58, Jiri Slaby wrote:
> On 18. 01. 26, 18:44, Osama Abdelkader wrote:
>> Add NULL pointer checks for ser and ser->dev in ldisc_receive() to
>> prevent memory leaks when the function is called during device close
>> or in race conditions where tty->disc_data or ser->dev may be NULL.
>>
>> The memory leak occurred because ser->dev was accessed before checking
>> if ser or ser->dev was NULL, which could cause a NULL pointer
>> dereference or use of freed memory. Additionally, set tty->disc_data
>> to NULL in ldisc_close() to prevent receive_buf() from using a freed
>> ser pointer after the line discipline is closed.
>>
>> Reported-by: syzbot+f9d847b2b84164fa69f3@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=f9d847b2b84164fa69f3
>> Fixes: 9b27105b4a44 ("net-caif-driver: add CAIF serial driver (ldisc)")
>> CC: stable@vger.kernel.org
>> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
>> ---
>> v2:
>> 1.Combine NULL pointer checks for ser and ser->dev in ldisc_receive()
>> 2.Set tty->disc_data = NULL in ldisc_close() to prevent receive_buf()
>> from using a freed ser pointer after close.
>> 3.Add NULL pointer check for ser in ldisc_close()
>> ---
>> drivers/net/caif/caif_serial.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/
>> caif_serial.c
>> index c398ac42eae9..970237a3ccca 100644
>> --- a/drivers/net/caif/caif_serial.c
>> +++ b/drivers/net/caif/caif_serial.c
>> @@ -152,6 +152,8 @@ static void ldisc_receive(struct tty_struct *tty,
>> const u8 *data,
>> int ret;
>> ser = tty->disc_data;
>> + if (!ser || !ser->dev)
>> + return;
>> /*
>> * NOTE: flags may contain information about break or overrun.
>> @@ -170,8 +172,6 @@ static void ldisc_receive(struct tty_struct *tty,
>> const u8 *data,
>> return;
>> }
>> - BUG_ON(ser->dev == NULL);
>> -
>> /* Get a suitable caif packet and copy in data. */
>> skb = netdev_alloc_skb(ser->dev, count+1);
>
> Wait, the reported error is memory leak of mem allocated here. So both
> ser and ser->dev appear to be valid?
>
> So instead, does netif_rx() return an error few lines below for some
> reason? So should skb just be freed in that path?
>
> if (skb == NULL)
> return;
> skb_put_data(skb, data, count);
>
> skb->protocol = htons(ETH_P_CAIF);
> skb_reset_mac_header(skb);
> debugfs_rx(ser, data, count);
> /* Push received packet up the stack. */
> ret = netif_rx(skb);
> if (!ret) {
> ser->dev->stats.rx_packets++;
> ser->dev->stats.rx_bytes += count;
> } else
> ++ser->dev->stats.rx_dropped;
>
> Not calling skb_free() in this else path _is_ a BUG™ in any case IMO.
No, it's not: netif_rx() always eats the buffer. So care to elaborate
why the socket buffer is actually leaked?
> thanks,
--
js
suse labs
On Mon, Jan 19, 2026 at 08:00:37AM +0100, Jiri Slaby wrote:
> On 19. 01. 26, 7:58, Jiri Slaby wrote:
> > On 18. 01. 26, 18:44, Osama Abdelkader wrote:
> > > Add NULL pointer checks for ser and ser->dev in ldisc_receive() to
> > > prevent memory leaks when the function is called during device close
> > > or in race conditions where tty->disc_data or ser->dev may be NULL.
> > >
> > > The memory leak occurred because ser->dev was accessed before checking
> > > if ser or ser->dev was NULL, which could cause a NULL pointer
> > > dereference or use of freed memory. Additionally, set tty->disc_data
> > > to NULL in ldisc_close() to prevent receive_buf() from using a freed
> > > ser pointer after the line discipline is closed.
> > >
> > > Reported-by: syzbot+f9d847b2b84164fa69f3@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=f9d847b2b84164fa69f3
> > > Fixes: 9b27105b4a44 ("net-caif-driver: add CAIF serial driver (ldisc)")
> > > CC: stable@vger.kernel.org
> > > Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> > > ---
> > > v2:
> > > 1.Combine NULL pointer checks for ser and ser->dev in ldisc_receive()
> > > 2.Set tty->disc_data = NULL in ldisc_close() to prevent receive_buf()
> > > from using a freed ser pointer after close.
> > > 3.Add NULL pointer check for ser in ldisc_close()
> > > ---
> > > drivers/net/caif/caif_serial.c | 8 ++++++--
> > > 1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/
> > > caif_serial.c
> > > index c398ac42eae9..970237a3ccca 100644
> > > --- a/drivers/net/caif/caif_serial.c
> > > +++ b/drivers/net/caif/caif_serial.c
> > > @@ -152,6 +152,8 @@ static void ldisc_receive(struct tty_struct
> > > *tty, const u8 *data,
> > > int ret;
> > > ser = tty->disc_data;
> > > + if (!ser || !ser->dev)
> > > + return;
> > > /*
> > > * NOTE: flags may contain information about break or overrun.
> > > @@ -170,8 +172,6 @@ static void ldisc_receive(struct tty_struct
> > > *tty, const u8 *data,
> > > return;
> > > }
> > > - BUG_ON(ser->dev == NULL);
> > > -
> > > /* Get a suitable caif packet and copy in data. */
> > > skb = netdev_alloc_skb(ser->dev, count+1);
> >
> > Wait, the reported error is memory leak of mem allocated here. So both
> > ser and ser->dev appear to be valid?
> >
> > So instead, does netif_rx() return an error few lines below for some
> > reason? So should skb just be freed in that path?
> >
> > if (skb == NULL)
> > return;
> > skb_put_data(skb, data, count);
> >
> > skb->protocol = htons(ETH_P_CAIF);
> > skb_reset_mac_header(skb);
> > debugfs_rx(ser, data, count);
> > /* Push received packet up the stack. */
> > ret = netif_rx(skb);
> > if (!ret) {
> > ser->dev->stats.rx_packets++;
> > ser->dev->stats.rx_bytes += count;
> > } else
> > ++ser->dev->stats.rx_dropped;
> >
> > Not calling skb_free() in this else path _is_ a BUG™ in any case IMO.
>
> No, it's not: netif_rx() always eats the buffer. So care to elaborate why
> the socket buffer is actually leaked?
>
> > thanks,
> --
> js
> suse labs
>
Hi Jiri, Greg
Following up on v2 feedback regarding the CAIF close-receive race,
I'm thinking of (similar to PPP driver):
- Add proper lifetime management using a refcount (`rx_ref`) to track
in-flight receive callbacks.
- Introduce a `closing` flag to prevent new receive callbacks after ldisc_close()
starts.
- Use a completion (`rx_done`) to wait for in-flight callbacks to finish before
freeing resources.
I plan to verify the patch using the ReproC program and kmemleak locally, but I
would appreciate feedback on this approach before testing.
proposal is written below.
Thanks for your review!
Best regards,
Osama
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ struct ser_device {
/* existing fields */
+ refcount_t rx_ref; /* track in-flight receive callbacks */
+ bool closing; /* indicates ldisc_close in progress */
+ spinlock_t lock; /* protects closing flag */
+ struct completion rx_done; /* signals all RX finished */
};
@@ static void ldisc_receive(struct tty_struct *tty,
struct ser_device *ser = tty->disc_data;
- if (!ser)
- return;
+ if (!ser)
+ return; /* safety check, usually handled by TTY core */
+
+ /* Acquire lock to check closing */
+ spin_lock(&ser->lock);
+ if (ser->closing) {
+ spin_unlock(&ser->lock);
+ return;
+ }
+ refcount_inc(&ser->rx_ref);
+ spin_unlock(&ser->lock);
+
+ /* existing RX processing code */
+
+ /* RX done */
+ if (refcount_dec_and_test(&ser->rx_ref))
+ complete(&ser->rx_done);
}
@@ static void ldisc_close(struct tty_struct *tty)
struct ser_device *ser = tty->disc_data;
if (!ser)
return;
+
+ /* mark closing and prevent new receive callbacks */
+ spin_lock(&ser->lock);
+ ser->closing = true;
+ spin_unlock(&ser->lock);
+
+ /* wait for in-flight RX to finish */
+ wait_for_completion(&ser->rx_done);
+
+ /* safe to free ser and associated resources now */
+
+ /* optional: re-init completion for future opens */
+ init_completion(&ser->rx_done);
On Sun, Jan 18, 2026 at 06:44:16PM +0100, Osama Abdelkader wrote:
> Add NULL pointer checks for ser and ser->dev in ldisc_receive() to
> prevent memory leaks when the function is called during device close
> or in race conditions where tty->disc_data or ser->dev may be NULL.
>
> The memory leak occurred because ser->dev was accessed before checking
> if ser or ser->dev was NULL, which could cause a NULL pointer
> dereference or use of freed memory. Additionally, set tty->disc_data
> to NULL in ldisc_close() to prevent receive_buf() from using a freed
> ser pointer after the line discipline is closed.
>
> Reported-by: syzbot+f9d847b2b84164fa69f3@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f9d847b2b84164fa69f3
> Fixes: 9b27105b4a44 ("net-caif-driver: add CAIF serial driver (ldisc)")
> CC: stable@vger.kernel.org
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
> v2:
> 1.Combine NULL pointer checks for ser and ser->dev in ldisc_receive()
> 2.Set tty->disc_data = NULL in ldisc_close() to prevent receive_buf()
> from using a freed ser pointer after close.
> 3.Add NULL pointer check for ser in ldisc_close()
I see no locking fixes, so I don't see how this will really work.
How do the other ldisc drivers handle this same issue?
thanks,
greg k-h
On 18. 01. 26, 18:44, Osama Abdelkader wrote:
> Add NULL pointer checks for ser and ser->dev in ldisc_receive() to
> prevent memory leaks when the function is called during device close
> or in race conditions where tty->disc_data or ser->dev may be NULL.
>
> The memory leak occurred because ser->dev was accessed before checking
> if ser or ser->dev was NULL, which could cause a NULL pointer
> dereference or use of freed memory. Additionally, set tty->disc_data
> to NULL in ldisc_close() to prevent receive_buf() from using a freed
> ser pointer after the line discipline is closed.
>
> Reported-by: syzbot+f9d847b2b84164fa69f3@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f9d847b2b84164fa69f3
> Fixes: 9b27105b4a44 ("net-caif-driver: add CAIF serial driver (ldisc)")
> CC: stable@vger.kernel.org
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
> v2:
> 1.Combine NULL pointer checks for ser and ser->dev in ldisc_receive()
> 2.Set tty->disc_data = NULL in ldisc_close() to prevent receive_buf()
> from using a freed ser pointer after close.
> 3.Add NULL pointer check for ser in ldisc_close()
> ---
> drivers/net/caif/caif_serial.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
> index c398ac42eae9..970237a3ccca 100644
> --- a/drivers/net/caif/caif_serial.c
> +++ b/drivers/net/caif/caif_serial.c
> @@ -152,6 +152,8 @@ static void ldisc_receive(struct tty_struct *tty, const u8 *data,
> int ret;
>
> ser = tty->disc_data;
> + if (!ser || !ser->dev)
NACK. If receive_buf is called with NULL tty->disc_data (outside of
ldisc open-close window), it's a bug one layer up. I.e. another race in
old bad tiocsti().
> + return;
>
> /*
> * NOTE: flags may contain information about break or overrun.
> @@ -170,8 +172,6 @@ static void ldisc_receive(struct tty_struct *tty, const u8 *data,
> return;
> }
>
> - BUG_ON(ser->dev == NULL);
> -
> /* Get a suitable caif packet and copy in data. */
> skb = netdev_alloc_skb(ser->dev, count+1);
> if (skb == NULL)
> @@ -355,11 +355,15 @@ static void ldisc_close(struct tty_struct *tty)
> {
> struct ser_device *ser = tty->disc_data;
>
> + if (!ser)
Meaning close can be called twice? Or without open? Really?
> + return;
> +
> tty_kref_put(ser->tty);
>
> spin_lock(&ser_lock);
> list_move(&ser->node, &ser_release_list);
> spin_unlock(&ser_lock);
> + tty->disc_data = NULL;
This wouldn't hurt, but should not be needed since:
dd42bf119714 tty: Prevent ldisc drivers from re-using stale tty fields
thanks,
--
js
suse labs
© 2016 - 2026 Red Hat, Inc.