Message-ID: <1499089066.1935.77.camel@codethink.co.uk>
Subject: Re: [PATCH 4.4 16/30] [media] pvrusb2: reduce stack usage
 pvr2_eeprom_analyze()
From: Ben Hutchings <ben.hutchings@codethink.co.uk>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "# 3.4.x" <stable@vger.kernel.org>,
        Hans Verkuil <hans.verkuil@cisco.com>,
        Mauro Carvalho Chehab <mchehab@s-opensource.com>
Date: Mon, 03 Jul 2017 14:37:46 +0100
In-Reply-To: <CAK8P3a0TAfBAZ8yWxJZwxDshB1ck1yn=VOXtM6D=cko=9GVAnA@mail.gmail.com>
References: <20170619152033.211450261@linuxfoundation.org>
         <20170619152034.086147988@linuxfoundation.org>
         <1498752917.1935.52.camel@codethink.co.uk>
         <20170703073153.GE5888@kroah.com>
         <CAK8P3a0TAfBAZ8yWxJZwxDshB1ck1yn=VOXtM6D=cko=9GVAnA@mail.gmail.com>
Organization: Codethink Ltd.
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2296
Lines: 58

On Mon, 2017-07-03 at 09:47 +0200, Arnd Bergmann wrote:
> On Mon, Jul 3, 2017 at 9:31 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> 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 <arnd@arndb.de>
> >> >
> >> > commit 6830733d53a4517588e56227b9c8538633f0c496 upstream.
[...]
> >> > 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 <mchehab@s-opensource.com>
> >> 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.

Sorry, I mixed up two commits.  The one actually needed to avoid a null
dereference is:

commit 7cda4c5bae46ffca3abeadc4c1882d9325ee3102
Author: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Date:   Thu Oct 13 10:51:20 2016 -0300

    [media] tveeprom: use dev_foo() for printk messages

Then "[media] tveeprom: print log messages using pr_foo()" is cleanup on
top of that.

Ben.

-- 
Ben Hutchings
Software Developer, Codethink Ltd.