Received: by 2002:a05:6a10:f347:0:0:0:0 with SMTP id d7csp3387536pxu; Mon, 30 Nov 2020 01:46:59 -0800 (PST) X-Google-Smtp-Source: ABdhPJxmzPC+77E9wQkj7lKv65RkG5vqyHoq1FzyBuyOcI32mTw3totR23FhhvCBzZrEp/JnzJCe X-Received: by 2002:a50:e882:: with SMTP id f2mr19468401edn.76.1606729619724; Mon, 30 Nov 2020 01:46:59 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1606729619; cv=none; d=google.com; s=arc-20160816; b=wlA5QU6PhhpKqSsTk80DUPZ4cv0ijENKNw5NMj2rTCYSyUuLpvCBINYgTqgtishvWV uLXpNfc0gf2NHPDNKHC4D4vQMDXq4Zagmsk8VY3Y6AeXNyZ/5UiIgaFkM7S9d2F+PfZd 10c4apj25hL5qEjXGlvJfHZ6M5nOjPeIeXulv19nxZGAL2+YaTRdJdG86qGgTu5LYayx fFkZ/2QaXrwOWUsM1qCck7UCrcFhit+2sBQweuIBS616L7nWjDEX3U+8XcuxdrzfqA+B Bc5DFiK8pc8Jrko4ua6/eEHBaZVjLx4nBMBotLkvsojUWSn474Tne9T/PIRemracIDl1 X4pA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date :dkim-signature; bh=ISKzSGOwD0qNMhSzD9b2pAOXv0JUshly3PXL6PztzhI=; b=rZQ9yIpZ+/Vk/8FsYr+XNoqQGodQkyM5b/5Asx0tH4azKbFAZKrtWsyWiRp7FYFBFg GXct4s411OaxZ+h53iTPyRZATJiKvMsh/5F9pWuQWqBr6PokfwVIFhoK/lXHwcTc2ojf Ar6XkbDyLwao1LK9wAXiIZZM23KmBb8PcoELWnALme91k2gEiAv1k0QjMZ1dsPIcEcC4 /WUVhCZqyG/FrrNRefIYnrjD0fbHAbvEnkKsTsEqRUE50Lbk677MeRhPRkG32EAClPyC o5/B4otSMNXWdjn30LX/PyOGKWzY/tU6bxLcrWeSeWPRgFFXyiI71B1hujbVBlDyqVsC e8ZA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=Itxz+yl5; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id q7si10901074edc.249.2020.11.30.01.46.35; Mon, 30 Nov 2020 01:46:59 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=Itxz+yl5; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728332AbgK3JpH (ORCPT + 99 others); Mon, 30 Nov 2020 04:45:07 -0500 Received: from mail.kernel.org ([198.145.29.99]:54648 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725972AbgK3JpG (ORCPT ); Mon, 30 Nov 2020 04:45:06 -0500 Received: from coco.lan (ip5f5ad5b3.dynamic.kabel-deutschland.de [95.90.213.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 7925020809; Mon, 30 Nov 2020 09:44:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1606729465; bh=yB55frgZs6cTDNzWFHRr6G9akT3+WYu/zA7dYKpAh10=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Itxz+yl5mLeHTBFE9VzNhtyri/edN22LDBjk0eg5SMwzTJZyxGAe3kR9cikLp7Kq6 WEWq4oQeJbn1xpoWR8sC9TJjPckXdVcX+FDqums+zth4qUvY+2QmA49y113aj5rAQO AfSmIT/cgJeDfIPIrMxHuZOPIntpvk0R9uvm7fv4= Date: Mon, 30 Nov 2020 10:44:20 +0100 From: Mauro Carvalho Chehab To: Willem de Bruijn Cc: Linux Media Mailing List , linuxarm@huawei.com, mauro.chehab@huawei.com, Mauro Carvalho Chehab , linux-kernel , syzbot Subject: Re: [PATCH] media: gp8psk: initialize stats at power control logic Message-ID: <20201130104420.321531ec@coco.lan> In-Reply-To: References: X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.32; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Fri, 27 Nov 2020 09:20:53 -0500 Willem de Bruijn escreveu: > On Fri, Nov 27, 2020 at 1:46 AM Mauro Carvalho Chehab > wrote: > > > > As reported on: > > https://lore.kernel.org/linux-media/20190627222020.45909-1-willemdebruijn.kernel@gmail.com/ > > > > if gp8psk_usb_in_op() returns an error, the status var is not > > initialized. Yet, this var is used later on, in order to > > identify: > > - if the device was already started; > > - if firmware has loaded; > > - if the LNBf was powered on. > > > > Using status = 0 seems to ensure that everything will be > > properly powered up. > > > > So, instead of the proposed solution, let's just set > > status = 0. > > > > Reported-by: syzbot > > Reported-by: Willem de Bruijn > > Signed-off-by: Mauro Carvalho Chehab > > --- > > drivers/media/usb/dvb-usb/gp8psk.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/media/usb/dvb-usb/gp8psk.c b/drivers/media/usb/dvb-usb/gp8psk.c > > index c07f46f5176e..b4f661bb5648 100644 > > --- a/drivers/media/usb/dvb-usb/gp8psk.c > > +++ b/drivers/media/usb/dvb-usb/gp8psk.c > > @@ -182,7 +182,7 @@ static int gp8psk_load_bcm4500fw(struct dvb_usb_device *d) > > > > static int gp8psk_power_ctrl(struct dvb_usb_device *d, int onoff) > > { > > - u8 status, buf; > > + u8 status = 0, buf; > > int gp_product_id = le16_to_cpu(d->udev->descriptor.idProduct); > > > > if (onoff) { > > -- > > 2.28.0 > > > Is it okay to ignore the return value of gp8psk_usb_in_op here? Well, I don't have this specific hardware in my hands, but, if you follow the logic there, it sounds ok to ignore. It should be noticed that, on some devices, some I2C commands will only return after having the device powered up and its firmware loaded. As this code is at the powerup part of the code, it sound reasonable to assume that the I2C read might fail. So, this change is less aggressive than just returning, as the driver may be relying on a fail read already. --- If you follow the logic of this routine, a fail to read means that the hardware is not able to return to this specific I2C command, either because it was physically (or logically) removed or because it was not properly powered up. If it was removed, trying to send I2C commands to power it up will return errors, so the first attempt of writing data to it will return an error. If, on the other hand, the hardware was not properly powered up, status = 0 will mean that all parts of the chipset should be powered on. As this is the only place at the driver where a read is not checked for its success, I'm assuming that this is the original intent of the driver's author. Thanks, Mauro