linux-kernel - Re: [PATCH 5/5] staging: r8188eu: Use vsprintf extension %phCX to format a copy_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52ab858d1061bd8c0da8bba247235d92a5e4a0ec.camel@perches.com>
Date:   Fri, 27 Aug 2021 08:52:36 -0700
From:   Joe Perches <joe@...ches.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org,
        Larry Finger <Larry.Finger@...inger.net>,
        Phillip Potter <phil@...lpotter.co.uk>,
        linux-staging@...ts.linux.dev
Subject: Re: [PATCH 5/5] staging: r8188eu: Use vsprintf extension %phCX to
 format a copy_to_user string

On Fri, 2021-08-27 at 17:27 +0200, Greg Kroah-Hartman wrote:
> On Fri, Aug 27, 2021 at 08:23:31AM -0700, Joe Perches wrote:
> > On Fri, 2021-08-27 at 10:42 +0200, Greg Kroah-Hartman wrote:
> > > On Thu, Aug 26, 2021 at 11:43:05AM -0700, Joe Perches wrote:
> > > > This reduces object size without changing the string content.
[]
> > > > diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
[]
> > The copy_to_user bit (2nd diff block) is nominally an ABI and is upper case.
> > IMO at a minimum, it's bad form to change it.
> > 
> > @@ -3075,8 +3073,7 @@ static int rtw_p2p_get_go_device_address(struct net_device *dev,
> >         if (!blnMatch)
> >                 sprintf(go_devadd_str, "\n\ndev_add = NULL");
> >         else
> > -               sprintf(go_devadd_str, "\ndev_add =%.2X:%.2X:%.2X:%.2X:%.2X:%.2X",
> > -                       attr_content[0], attr_content[1], attr_content[2], attr_content[3], attr_content[4], attr_content[5]);
> > +               sprintf(go_devadd_str, "\ndev_add =%6phCX", attr_content);
> > 
> >         if (copy_to_user(wrqu->data.pointer, go_devadd_str, 10 + 17))
> >                 return -EFAULT;
> 
> That looks like a horrible driver-specific api

Horrible could be used to describe most realtek code in the kernel.

> that no one will really
> be using and will be removed from the tree soon.  So it can be changed,
> no need to worry about any "compatibility" issues here.

Fine with me too.  I just did it for completeness.