Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752825AbdGCHru (ORCPT ); Mon, 3 Jul 2017 03:47:50 -0400 Received: from mail-oi0-f65.google.com ([209.85.218.65]:34781 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752334AbdGCHro (ORCPT ); Mon, 3 Jul 2017 03:47:44 -0400 MIME-Version: 1.0 In-Reply-To: <20170703073153.GE5888@kroah.com> References: <20170619152033.211450261@linuxfoundation.org> <20170619152034.086147988@linuxfoundation.org> <1498752917.1935.52.camel@codethink.co.uk> <20170703073153.GE5888@kroah.com> From: Arnd Bergmann Date: Mon, 3 Jul 2017 09:47:43 +0200 X-Google-Sender-Auth: us8pDyTCTDG3afpHZND1rQ3gB9s Message-ID: Subject: Re: [PATCH 4.4 16/30] [media] pvrusb2: reduce stack usage pvr2_eeprom_analyze() To: Greg Kroah-Hartman Cc: Ben Hutchings , Linux Kernel Mailing List , "# 3.4.x" , Hans Verkuil , Mauro Carvalho Chehab Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2307 Lines: 51 On Mon, Jul 3, 2017 at 9:31 AM, Greg Kroah-Hartman wrote: > On Thu, Jun 29, 2017 at 05:15:17PM +0100, Ben Hutchings wrote: >> On Mon, 2017-06-19 at 23:20 +0800, Greg Kroah-Hartman wrote: >> > 4.4-stable review patch. If anyone has any objections, please let me know. >> > >> > From: Arnd Bergmann >> > >> > commit 6830733d53a4517588e56227b9c8538633f0c496 upstream. >> > >> > The driver uses a relatively large data structure on the stack, which >> > showed up on my radar as we get a warning with the "latent entropy" >> > GCC plugin: >> > >> > drivers/media/usb/pvrusb2/pvrusb2-eeprom.c:153:1: error: the frame size of 1376 bytes is larger than 1152 bytes [-Werror=frame-larger-than=] >> > >> > The warning is usually hidden as we raise the warning limit to 2048 >> > when the plugin is enabled, but I'd like to lower that again in the >> > future, and making this function smaller helps to do that without >> > build regressions. >> > >> > Further analysis shows that putting an 'i2c_client' structure on >> > the stack is not really supported, as the embedded 'struct device' >> > is not initialized here, and we are only saved by the fact that >> > the function that is called here does not use the pointer at all. >> [...] >> >> That is not true in 4.4-stable. This commit depends on: >> >> commit 6037b3ca28f4258d913dbe77248fd77827702ae3 >> Author: Mauro Carvalho Chehab >> Date: Wed Nov 16 14:21:48 2016 -0200 >> >> [media] tveeprom: print log messages using pr_foo() > > It does? I don't understand how the two are connected. Removing > i2c_client off of the stack is a good thing. Ah, I see how the pointer > is used in tveeprom_hauppauge_analog(), but this shouldn't matter here, > right? My reading of the two patches is that we actually need at least one of them to avoid interpreting uninitialized dev->class/bus: With just my 6830733d53a patch, we replace the uninitialized data with a NULL pointer, which is handled gracefully in __dev_printk(), while the 6037b3ca28 patch by itself will avoid using the 'dev' pointer completely, and give a saner output (no "(NULL device)" string or worse). I think we probably want both of them backported to 4.4, but I don't see a dependency between them. Arnd