[PATCH v2] Staging: emxx_udc: Make a read-only structure

[PATCH v2] Staging: emxx_udc: Make a read-only structure

[Mihaela Muraru]

This patch add const qualifier at the declaration of the structure,
because it contains information about usb hardware device and those
values shoud not be overwritten.

The structure become a read-only data, this increase the security and
the integrity of the program.

Found with Coccinelle:
@r disable optional_qualifier@
identifier s,i;
@@
* static struct s i ={...};

Signed-off-by: Mihaela Muraru <mihaela.muraru21@gmail.com>
---
 drivers/staging/emxx_udc/emxx_udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
index c3e2988..8b406f3 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -1386,7 +1386,7 @@ static void _nbu2ss_set_endpoint_stall(
 
 /*-------------------------------------------------------------------------*/
 /* Device Descriptor */
-static struct usb_device_descriptor device_desc = {
+static const struct usb_device_descriptor device_desc = {
 	.bLength              = sizeof(device_desc),
 	.bDescriptorType      = USB_DT_DEVICE,
 	.bcdUSB               = cpu_to_le16(0x0200),
-- 
2.7.4

Re: [Outreachy kernel] [PATCH v2] Staging: emxx_udc: Make a read-only structure

[Julia Lawall]

On Tue, 18 Oct 2016, Mihaela Muraru wrote:

> This patch add const qualifier at the declaration of the structure,
> because it contains information about usb hardware device and those
> values shoud not be overwritten.

This focuses on the intent, which makes sense, but one really wants to be
sure that we won't end up with a const value floating into a modification
context, which would cause a runtime error.  So it would be good to
describe how the structure is used.  For example, is it only passed to
another function where the corresponding parameter is declared as const?

julia

>
> The structure become a read-only data, this increase the security and
> the integrity of the program.
>
> Found with Coccinelle:
> @r disable optional_qualifier@
> identifier s,i;
> @@
> * static struct s i ={...};
>
> Signed-off-by: Mihaela Muraru <mihaela.muraru21@gmail.com>
> ---
>  drivers/staging/emxx_udc/emxx_udc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
> index c3e2988..8b406f3 100644
> --- a/drivers/staging/emxx_udc/emxx_udc.c
> +++ b/drivers/staging/emxx_udc/emxx_udc.c
> @@ -1386,7 +1386,7 @@ static void _nbu2ss_set_endpoint_stall(
>
>  /*-------------------------------------------------------------------------*/
>  /* Device Descriptor */
> -static struct usb_device_descriptor device_desc = {
> +static const struct usb_device_descriptor device_desc = {
>  	.bLength              = sizeof(device_desc),
>  	.bDescriptorType      = USB_DT_DEVICE,
>  	.bcdUSB               = cpu_to_le16(0x0200),
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20161018204221.GA7799%40domino-MS-16Y1.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH v2] Staging: emxx_udc: Make a read-only structure

[Muraru Mihaela]

On Tue, Oct 18, 2016 at 10:48:54PM +0200, Julia Lawall wrote:
> On Tue, 18 Oct 2016, Mihaela Muraru wrote:
> 
> > This patch add const qualifier at the declaration of the structure,
> > because it contains information about usb hardware device and those
> > values shoud not be overwritten.
> 
> This focuses on the intent, which makes sense, but one really wants to be
> sure that we won't end up with a const value floating into a modification
> context, which would cause a runtime error.  So it would be good to
> describe how the structure is used.  For example, is it only passed to
> another function where the corresponding parameter is declared as const?

The structure is declared and used only in emxx_udc.c. I searched it in all
staging driver.

And is passed as a patameter only to the sizeof operetor, 
that doesn't change the variable of the structure.

To be 100% sure I will search on all the files of the kernel,
it would take some time, and I will tell you what is the result, if is
ok for you. 

mihaela


> 
> >
> > The structure become a read-only data, this increase the security and
> > the integrity of the program.
> >
> > Found with Coccinelle:
> > @r disable optional_qualifier@
> > identifier s,i;
> > @@
> > * static struct s i ={...};
> >
> > Signed-off-by: Mihaela Muraru <mihaela.muraru21@gmail.com>
> > ---
> >  drivers/staging/emxx_udc/emxx_udc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
> > index c3e2988..8b406f3 100644
> > --- a/drivers/staging/emxx_udc/emxx_udc.c
> > +++ b/drivers/staging/emxx_udc/emxx_udc.c
> > @@ -1386,7 +1386,7 @@ static void _nbu2ss_set_endpoint_stall(
> >
> >  /*-------------------------------------------------------------------------*/
> >  /* Device Descriptor */
> > -static struct usb_device_descriptor device_desc = {
> > +static const struct usb_device_descriptor device_desc = {
> >  	.bLength              = sizeof(device_desc),
> >  	.bDescriptorType      = USB_DT_DEVICE,
> >  	.bcdUSB               = cpu_to_le16(0x0200),
> > --
> > 2.7.4
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20161018204221.GA7799%40domino-MS-16Y1.
> > For more options, visit https://groups.google.com/d/optout.
> >

Re: [Outreachy kernel] [PATCH v2] Staging: emxx_udc: Make a read-only structure

[Julia Lawall]

On Wed, 19 Oct 2016, Muraru Mihaela wrote:

> On Tue, Oct 18, 2016 at 10:48:54PM +0200, Julia Lawall wrote:
> > On Tue, 18 Oct 2016, Mihaela Muraru wrote:
> >
> > > This patch add const qualifier at the declaration of the structure,
> > > because it contains information about usb hardware device and those
> > > values shoud not be overwritten.
> >
> > This focuses on the intent, which makes sense, but one really wants to be
> > sure that we won't end up with a const value floating into a modification
> > context, which would cause a runtime error.  So it would be good to
> > describe how the structure is used.  For example, is it only passed to
> > another function where the corresponding parameter is declared as const?
>
> The structure is declared and used only in emxx_udc.c. I searched it in all
> staging driver.
>
> And is passed as a patameter only to the sizeof operetor,
> that doesn't change the variable of the structure.
>
> To be 100% sure I will search on all the files of the kernel,
> it would take some time, and I will tell you what is the result, if is
> ok for you.

If the structure is static, then there is normally no need to check other
files.  The question though is how is it used in the current file.  Sizeof
is indeed not an issue, but what are the other ways in which it is used.

julia

Re: [Outreachy kernel] [PATCH v2] Staging: emxx_udc: Make a read-only structure

[Muraru Mihaela]

On Wed, Oct 19, 2016 at 08:01:40AM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 19 Oct 2016, Muraru Mihaela wrote:
> 
> > On Tue, Oct 18, 2016 at 10:48:54PM +0200, Julia Lawall wrote:
> > > On Tue, 18 Oct 2016, Mihaela Muraru wrote:
> > >
> > > > This patch add const qualifier at the declaration of the structure,
> > > > because it contains information about usb hardware device and those
> > > > values shoud not be overwritten.
> > >
> > > This focuses on the intent, which makes sense, but one really wants to be
> > > sure that we won't end up with a const value floating into a modification
> > > context, which would cause a runtime error.  So it would be good to
> > > describe how the structure is used.  For example, is it only passed to
> > > another function where the corresponding parameter is declared as const?
> >
> > The structure is declared and used only in emxx_udc.c. I searched it in all
> > staging driver.
> >
> > And is passed as a patameter only to the sizeof operetor,
> > that doesn't change the variable of the structure.
> >
> > To be 100% sure I will search on all the files of the kernel,
> > it would take some time, and I will tell you what is the result, if is
> > ok for you.
> 
> If the structure is static, then there is normally no need to check other
> files.  The question though is how is it used in the current file.  Sizeof
> is indeed not an issue, but what are the other ways in which it is used.

As I mentioned  it is not used in the current file. Now, I am
wondering why is declared.

mihaela
> julia

Re: [Outreachy kernel] [PATCH v2] Staging: emxx_udc: Make a read-only structure

[Greg Kroah-Hartman]

On Wed, Oct 19, 2016 at 09:18:29AM +0300, Muraru Mihaela wrote:
> On Wed, Oct 19, 2016 at 08:01:40AM +0200, Julia Lawall wrote:
> > 
> > 
> > On Wed, 19 Oct 2016, Muraru Mihaela wrote:
> > 
> > > On Tue, Oct 18, 2016 at 10:48:54PM +0200, Julia Lawall wrote:
> > > > On Tue, 18 Oct 2016, Mihaela Muraru wrote:
> > > >
> > > > > This patch add const qualifier at the declaration of the structure,
> > > > > because it contains information about usb hardware device and those
> > > > > values shoud not be overwritten.
> > > >
> > > > This focuses on the intent, which makes sense, but one really wants to be
> > > > sure that we won't end up with a const value floating into a modification
> > > > context, which would cause a runtime error.  So it would be good to
> > > > describe how the structure is used.  For example, is it only passed to
> > > > another function where the corresponding parameter is declared as const?
> > >
> > > The structure is declared and used only in emxx_udc.c. I searched it in all
> > > staging driver.
> > >
> > > And is passed as a patameter only to the sizeof operetor,
> > > that doesn't change the variable of the structure.
> > >
> > > To be 100% sure I will search on all the files of the kernel,
> > > it would take some time, and I will tell you what is the result, if is
> > > ok for you.
> > 
> > If the structure is static, then there is normally no need to check other
> > files.  The question though is how is it used in the current file.  Sizeof
> > is indeed not an issue, but what are the other ways in which it is used.
> 
> As I mentioned  it is not used in the current file. Now, I am
> wondering why is declared.

Yes, that is odd, does the compiler give you a warning about it when you
build the code that it's not being used?

thanks,

greg k-h

Re: [Outreachy kernel] [PATCH v2] Staging: emxx_udc: Make a read-only structure

[Muraru Mihaela]

On Wed, Oct 19, 2016 at 09:35:29AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 19, 2016 at 09:18:29AM +0300, Muraru Mihaela wrote:
> > On Wed, Oct 19, 2016 at 08:01:40AM +0200, Julia Lawall wrote:
> > > 
> > > 
> > > On Wed, 19 Oct 2016, Muraru Mihaela wrote:
> > > 
> > > > On Tue, Oct 18, 2016 at 10:48:54PM +0200, Julia Lawall wrote:
> > > > > On Tue, 18 Oct 2016, Mihaela Muraru wrote:
> > > > >
> > > > > > This patch add const qualifier at the declaration of the structure,
> > > > > > because it contains information about usb hardware device and those
> > > > > > values shoud not be overwritten.
> > > > >
> > > > > This focuses on the intent, which makes sense, but one really wants to be
> > > > > sure that we won't end up with a const value floating into a modification
> > > > > context, which would cause a runtime error.  So it would be good to
> > > > > describe how the structure is used.  For example, is it only passed to
> > > > > another function where the corresponding parameter is declared as const?
> > > >
> > > > The structure is declared and used only in emxx_udc.c. I searched it in all
> > > > staging driver.
> > > >
> > > > And is passed as a patameter only to the sizeof operetor,
> > > > that doesn't change the variable of the structure.
> > > >
> > > > To be 100% sure I will search on all the files of the kernel,
> > > > it would take some time, and I will tell you what is the result, if is
> > > > ok for you.
> > > 
> > > If the structure is static, then there is normally no need to check other
> > > files.  The question though is how is it used in the current file.  Sizeof
> > > is indeed not an issue, but what are the other ways in which it is used.
> > 
> > As I mentioned  it is not used in the current file. Now, I am
> > wondering why is declared.
> 
> Yes, that is odd, does the compiler give you a warning about it when you
> build the code that it's not being used?

No, I even delete it and is still compile, without  errors.


> thanks,
> 
> greg k-h