[PATCH v2] net: caif: fix memory leak in ldisc_receive

[PATCH v2] net: caif: fix memory leak in ldisc_receive

[Osama Abdelkader]

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

Re: [PATCH v2] net: caif: fix memory leak in ldisc_receive

[Jiri Slaby]

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

Re: [PATCH v2] net: caif: fix memory leak in ldisc_receive

[Greg Kroah-Hartman]

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

Re: [PATCH v2] net: caif: fix memory leak in ldisc_receive

[Jiri Slaby]

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.

thanks,
-- 
js
suse labs

Re: [PATCH v2] net: caif: fix memory leak in ldisc_receive

[Jiri Slaby]

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

Re: [PATCH v2] net: caif: fix memory leak in ldisc_receive

[Osama Abdelkader]

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