All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Pavel Skripkin <paskripkin@gmail.com>, mchehab@kernel.org
Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	syzbot+a6969ef522a36d3344c9@syzkaller.appspotmail.com
Subject: Re: [PATCH v2] media: em28xx: add missing em28xx_close_extension
Date: Thu, 29 Jul 2021 11:45:19 +0200	[thread overview]
Message-ID: <3b1de658-ebf0-e45f-49ef-953e3467e7d4@xs4all.nl> (raw)
In-Reply-To: <20210721194307.12155-1-paskripkin@gmail.com>

On 21/07/2021 21:43, Pavel Skripkin wrote:
> If em28xx dev has ->dev_next pointer, we need to delete dev_next list node
> from em28xx_extension_devlist on disconnect to avoid UAF bugs and
> corrupted list bugs, since driver frees this pointer on disconnect.
> 
> Fixes: 1a23f81b7dc3 ("V4L/DVB (9979): em28xx: move usb probe code to a proper place")
> Reported-and-tested-by: syzbot+a6969ef522a36d3344c9@syzkaller.appspotmail.com
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
> 
> Changes in v2:
> 	Previous patch was completely broken. I've done some debugging
> 	again and found true root case of the reported bug.
> 
> ---
>  drivers/media/usb/em28xx/em28xx-cards.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> index c1e0dccb7408..d56b040e1bd7 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -4139,8 +4139,10 @@ static void em28xx_usb_disconnect(struct usb_interface *intf)
>  
>  	em28xx_close_extension(dev);
>  
> -	if (dev->dev_next)
> +	if (dev->dev_next) {
>  		em28xx_release_resources(dev->dev_next);
> +		em28xx_close_extension(dev->dev_next);

Wouldn't it be better to swap these two?

That order is also used for em28xx_close_extension(dev) and em28xx_release_resources(dev).

You do need to store dev->dev_next in a temp variable, though.

Regards,

	Hans

> +	}
>  	em28xx_release_resources(dev);
>  
>  	if (dev->dev_next) {
> 


  reply	other threads:[~2021-07-29  9:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-06 14:50 [PATCH] media: em28xx: fix corrupted list Pavel Skripkin
2021-07-20 12:13 ` Hans Verkuil
2021-07-21 12:21   ` Pavel Skripkin
2021-07-21 12:25     ` Hans Verkuil
2021-07-21 19:43       ` [PATCH v2] media: em28xx: add missing em28xx_close_extension Pavel Skripkin
2021-07-29  9:45         ` Hans Verkuil [this message]
2021-07-29 12:45           ` Pavel Skripkin
2021-07-29 13:40             ` Hans Verkuil
2021-07-29 20:23               ` [PATCH v3] " Pavel Skripkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3b1de658-ebf0-e45f-49ef-953e3467e7d4@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=paskripkin@gmail.com \
    --cc=syzbot+a6969ef522a36d3344c9@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.