From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8B3AAC433EF for ; Sat, 15 Jan 2022 18:11:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233375AbiAOSLC (ORCPT ); Sat, 15 Jan 2022 13:11:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50482 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231278AbiAOSLB (ORCPT ); Sat, 15 Jan 2022 13:11:01 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A5F74C061574; Sat, 15 Jan 2022 10:11:01 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 5E64AB80A89; Sat, 15 Jan 2022 18:11:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AA03BC36AE7; Sat, 15 Jan 2022 18:10:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1642270259; bh=2PWywdObtfGp7J3+7ngcsnTM6kfub2PBBTbBnzm4lhE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=s3eJH/ugwNk+p8MbDxFSC9LnvnGfzEsg4QII/QRBsvsG+LEOx/PuNeSj6b60sS/Yy BG9Je6wEtPmHM9ROXxw9QGXrUKeZgVZ5OX+y/CGwneG+mr5VOUlDjk5zeTQGAmYkOL HouggdMpMT8ZCr6LlYrSCFoRgJm4bzJs8qJ+cXD0GxbbSnaE95rX9CJiDOxyVTM1V+ qiNS1D9bHHj0FGH7hkjf28pN8RDigxWCTbOSgoRN17xq6/P4tMBpYrXp39AZRlsmzn yUeW9tJbIzWLg+1J2qbRiIwBlTYgd0AwffFN6tf+vK5W/UEWja61rGZRWk6gJJN+a+ Oipiw7V9To3tg== Date: Sat, 15 Jan 2022 18:16:59 +0000 From: Jonathan Cameron To: Oleksij Rempel Cc: devicetree@vger.kernel.org, Lars-Peter Clausen , linux-iio@vger.kernel.org, Robin van der Gracht , linux-kernel@vger.kernel.org, Pengutronix Kernel Team , David Jander Subject: Re: [PATCH v1 1/1] iio: adc: tsc2046: fix memory corruption by preventing array overflow Message-ID: <20220115181659.0c759ec5@jic23-huawei> In-Reply-To: <20220110071945.GB3326@pengutronix.de> References: <20220107081401.2816357-1-o.rempel@pengutronix.de> <20220109152557.74f06d2d@jic23-huawei> <20220110071945.GB3326@pengutronix.de> X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Mon, 10 Jan 2022 08:19:45 +0100 Oleksij Rempel wrote: > Hi Jonathan, > > On Sun, Jan 09, 2022 at 03:25:57PM +0000, Jonathan Cameron wrote: > > On Fri, 7 Jan 2022 09:14:01 +0100 > > Oleksij Rempel wrote: > > > > > On one side we have indio_dev->num_channels includes all physical channels + > > > timestamp channel. On other side we have an array allocated only for > > > physical channels. So, fix memory corruption by ARRAY_SIZE() instead of > > > num_channels variable. > > > > > > Fixes: 9374e8f5a38d ("iio: adc: add ADC driver for the TI TSC2046 controller") > > > Signed-off-by: Oleksij Rempel > > Hi Olesij, > > > > Have you managed to make this occur, or is it inspection only? > > Yes, this bug has eaten my rx_one and tx_one pointers on probe. I wonted > to use this buffers for read_raw and noticed that they do not exist. I got hung up on the first case and failed to notice the second one was entirely different :( > > > I 'think' (it's been a while since I looked at the particular code) that the timestamp > > bit in active_scan_mask will never actually be set because we handle that as a > > separate flag. > > I didn't tested if active_scan_mask will trigger this issue as well, but > It it looked safer to me, to avoid this issue in both places. Even if on > of it is only theoretical. It certainly does no harm to not check a bit that is never set, so I'm fine with the change - just don't want to have lots of 'fixes' for this in other drivers adding noise and pointless backports. This one is fine because we need the other part of the patch anyway. Jonathan > > > So it is indeed an efficiency improvement to not check that bit but I don't think > > it's a bug to do so. More than possible I'm missing something though! > > > > This one had me quite worried when I first read it because this is a very common > > pattern to see in IIO drivers. > > I was thinking about this as well, because big part of this code was > inspired by other drivers. But i didn't reviewed other places so far. > > Regards, > Oleksij