[PATCH 1/1] gpiolib: Zero structure which will be returned to usermode to avoid kernel info leak.

[PATCH 1/1] gpiolib: Zero structure which will be returned to usermode to avoid kernel info leak.

[Gabriel Knezek]

Apologies if I got this slightly wrong; this is actually my first time submitting a patch to the kernel. (:

Thanks.
-Gabe

Fixes: aad95584 ("gpiolib: cdev: support GPIO_V2_GET_LINEINFO_IOCTL and GPIO_V2_GET_LINEINFO_WATCH_IOCTL")
Signed-off-by: Gabriel Knezek <gabeknez@microsoft.com>
---
 drivers/gpio/gpiolib-cdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index ee5903aac497..af68532835fe 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1865,6 +1865,7 @@ static void gpio_v2_line_info_changed_to_v1(
                struct gpio_v2_line_info_changed *lic_v2,
                struct gpioline_info_changed *lic_v1)
 {
+       memset(lic_v1, 0, sizeof(*lic_v1));
        gpio_v2_line_info_to_v1(&lic_v2->info, &lic_v1->info);
        lic_v1->timestamp = lic_v2->timestamp_ns;
        lic_v1->event_type = lic_v2->event_type;
--
2.25.1

Re: [PATCH 1/1] gpiolib: Zero structure which will be returned to usermode to avoid kernel info leak.

[Kent Gibson]

On Thu, Jun 17, 2021 at 08:56:50AM +0300, Andy Shevchenko wrote:
> On Thursday, June 17, 2021, Gabriel Knezek <gabeknez@microsoft.com> wrote:
> 
> > Apologies if I got this slightly wrong; this is actually my first time
> > submitting a patch to the kernel. (:
> >
> > Thanks.
> > -Gabe
> 
> 
> 
> Instead of above you should put a text of your analysis as you did in other
> thread. That text should be a commit message (assuming you have cloned a
> Git repository). `git format-patch ... ; git send-email ...` will help you
> with preparation and sending the message.
> 

I don't see Andy's reply on the list for some reason, so just to
re-iterate, you will need to issue a v2 patch.

The title could be a little more subtle ;).
The title should summarise what the change is, not the rationale nor
implications.  And mentioning the sub-module is handy too - in this case
anything mentioning cdev tends to get my attention.
So "gpiolib: cdev: zero padding during conversion to gpioline_info_changed"
would work better for me.

The checkin comment becomes part of the permanent record and so should
describe the rationale for the change - something more along the lines of
your initial thread, as Andy suggests.

Any additional commentary, generally related to the patch itself, goes
immediately after the "---" line - see below.  You should be able to
find plenty of examples on list.

You should use git to help format and generate your patch, as Andy
suggests, and run scripts/checkpatch.pl over it to make sure it looks
kosher before submitting it.

> >
> > Fixes: aad95584 ("gpiolib: cdev: support GPIO_V2_GET_LINEINFO_IOCTL and
> > GPIO_V2_GET_LINEINFO_WATCH_IOCTL")
> > Signed-off-by: Gabriel Knezek <gabeknez@microsoft.com>
> > ---

Additional commentary goes here.

e.g. 
Changes from v1: ....

This is explanatory stuff for the reviewers and maintainers that will
not become part of the git record.

As it isn't part of the git commit, this needs to be added after git
generates your patch.

> >  drivers/gpio/gpiolib-cdev.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> > index ee5903aac497..af68532835fe 100644
> > --- a/drivers/gpio/gpiolib-cdev.c
> > +++ b/drivers/gpio/gpiolib-cdev.c
> > @@ -1865,6 +1865,7 @@ static void gpio_v2_line_info_changed_to_v1(
> >                 struct gpio_v2_line_info_changed *lic_v2,
> >                 struct gpioline_info_changed *lic_v1)
> >  {
> > +       memset(lic_v1, 0, sizeof(*lic_v1));
> >         gpio_v2_line_info_to_v1(&lic_v2->info, &lic_v1->info);
> >         lic_v1->timestamp = lic_v2->timestamp_ns;
> >         lic_v1->event_type = lic_v2->event_type;
> > --
> > 2.25.1
> >
> 

I'm fine with the code change - just the formatting of the patch itself
needs to be tidied up.

Cheers,
Kent.