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: <DB9PR10MB58814F4711FC3403F7AF089BE0D19@DB9PR10MB5881.EURPRD10.PROD.OUTLOOK.COM>
Date:   Wed, 1 Feb 2023 09:17:31 +0000
From:   "Starke, Daniel" <daniel.starke@...mens.com>
To:     Greg KH <gregkh@...uxfoundation.org>
CC:     "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        "jirislaby@...nel.org" <jirislaby@...nel.org>,
        "ilpo.jarvinen@...ux.intel.com" <ilpo.jarvinen@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/3] tty: n_gsm: add keep alive support

> > index cb8693b39cb7..b64360aca1f9 100644
> > --- a/include/uapi/linux/gsmmux.h
> > +++ b/include/uapi/linux/gsmmux.h
> > @@ -19,7 +19,8 @@ struct gsm_config
> >  	unsigned int mtu;
> >  	unsigned int k;
> >  	unsigned int i;
> > -	unsigned int unused[8];		/* Padding for expansion without
> > +	unsigned int keep_alive;
> > +	unsigned int unused[7];		/* Padding for expansion without
> 
> "unsigned int" is not really a valid uapi variable type.
> 
> Shouldn't this be __u32 instead?

I know but changing it to a fixed size data type may break compatibility
as this may change the overall size of the structure. This is why I
took a field out of the "unused" array for the "keep_alive" parameter.
A value of zero disables keep-alive polling.

> Should you document this field as to what the value is and the units as
> you are creating a new user/kernel api here.

I will add a comment here. Comments for the other fields remain subject to
another patch.

> And finally, "unused" here is being properly checked to be all 0, right?
> If not, then this change can't happen for obvious reasons :(

This was not the case until now. I assumed there was some coding guideline
that unused fields need to be initialized to zero. Obviously, checking it
prevents misuse here. I will add relevant checks for this.

Best regards,
Daniel Starke

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ