Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 9 Jul 2022 09:55:41 +0800
From:   Kent Gibson <warthog618@gmail.com>
To:     Dipen Patel <dipenp@nvidia.com>
Cc:     Bartosz Golaszewski <brgl@bgdev.pl>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [PATCH] gpiolib: cdev: fix null pointer dereference in
 linereq_free()
Message-ID: <20220709015541.GA5332@sol>
References: <20220706084507.2259415-1-warthog618@gmail.com>
 <20220706085025.GA2259579@sol>
 <20220707090006.GB57165@sol>
 <CAMRc=Mcqvp4oN_tHXS1cb0AnEOU=Znd4bmSwO4ZZ1GszRfXhSA@mail.gmail.com>
 <20220707102918.GA60169@sol>
 <424af43a-bcaf-4f59-086d-b1afea425054@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <424af43a-bcaf-4f59-086d-b1afea425054@nvidia.com>
Precedence: bulk

On Fri, Jul 08, 2022 at 11:24:43AM -0700, Dipen Patel wrote:
> On 7/7/22 3:29 AM, Kent Gibson wrote:
> > On Thu, Jul 07, 2022 at 12:19:15PM +0200, Bartosz Golaszewski wrote:
> >> On Thu, Jul 7, 2022 at 11:00 AM Kent Gibson <warthog618@gmail.com> wrote:
> >>> On Wed, Jul 06, 2022 at 04:50:25PM +0800, Kent Gibson wrote:
> >>>> On Wed, Jul 06, 2022 at 04:45:07PM +0800, Kent Gibson wrote:
> >>>>> This patch fixes a kernel NULL pointer dereference that is reported by
> >>>>> gpio kselftests.
> >>>>>
> >>>> Should be:
> >>>>
> >>>> Fix a kernel NULL pointer dereference reported by gpio kselftests.
> >>>>
> >>>> Sorry - I rushed this one a bit.
> >>>>
> >>> And I might not've been totally clear, but this bug is present in
> >>> v5.19-rc1 onwards (when HTE was added), up to and including rc5.
> >>>
> >>> Would be good to get it fixed before v5.19 goes out the door.
> >>>
> >>> Cheers,
> >>> Kent.
> >> Yep, figured that out. Applied and fixed the commit message, thanks!
> >>
> > Good to hear.  I never got around to reviewing that final HTE patch
> > and, while it did end up pretty close to what I expected, there are a
> > few things that I would've done slightly differently that I'd like to
> > tidy up.
> I can create another thread to address this. Let me know.

I've already got the changes locally, so don't worry. There is nothing
critical or earth shattering in there, mainly how the hte_en flag is
passed around.  You followed the pattern provided by polarity_change,
which is only used on one occasion, when I would've followed the pattern
from line->eflags, which is used more widely.

Though I might take the changes a little further than I have to also
change the polarity_change flag to follow the eflags pattern to prevent
anyone following from falling into the same trap - still stewing on that.

The basic idea is that, particularly during reconfig, the struct line
contains the current applied state, while the desc->flags contain the
requested state.  The polarity_change flag was a bit of a lazy shortcut
to save from adding an active_low (or equivalent) flag to struct line.

> > And also have the HTE specific code compiled out unless CONFIG_HTE is
> > selected, as that is very likely to be the case for most builds.
> > But that can wait for v5.20.
> 
> I am assuming #ifdef CONFIG_HTE blocks around HTE blocks. I think Linus W. also
> 
> indicated that.
> 

Correct.

Cheers,
Kent.