[PATCH] [USB] UAS: Use kzalloc instead of kmalloc

[PATCH] [USB] UAS: Use kzalloc instead of kmalloc

[Luben Tuikov]

"Be conservative in what you send, liberal in what
you accept." -- In the spirit of this adage, don't
send Command IUs with randomly filled in data in
the reserved fields. (Yes, this shows up on the
wire.)

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 drivers/usb/storage/uas.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 2054b1e..17e1820 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -294,7 +294,7 @@ static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp,
 	if (!urb)
 		goto out;
 
-	iu = kmalloc(sizeof(*iu), gfp);
+	iu = kzalloc(sizeof(*iu), gfp);
 	if (!iu)
 		goto free;
 
@@ -325,7 +325,7 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp,
 	if (len < 0)
 		len = 0;
 	len = ALIGN(len, 4);
-	iu = kmalloc(sizeof(*iu) + len, gfp);
+	iu = kzalloc(sizeof(*iu) + len, gfp);
 	if (!iu)
 		goto free;
 
@@ -660,7 +660,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id)
 			return -ENODEV;
 	}
 
-	devinfo = kmalloc(sizeof(struct uas_dev_info), GFP_KERNEL);
+	devinfo = kzalloc(sizeof(struct uas_dev_info), GFP_KERNEL);
 	if (!devinfo)
 		return -ENOMEM;
 
-- 
1.7.0.1

Re: [PATCH] [USB] UAS: Use kzalloc instead of kmalloc

[Matthew Wilcox]

On Wed, Oct 27, 2010 at 01:22:22PM -0700, Luben Tuikov wrote:
> "Be conservative in what you send, liberal in what
> you accept." -- In the spirit of this adage, don't
> send Command IUs with randomly filled in data in
> the reserved fields. (Yes, this shows up on the
> wire.)

Applied, with a better changelog entry ...

> @@ -660,7 +660,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  			return -ENODEV;
>  	}
>  
> -	devinfo = kmalloc(sizeof(struct uas_dev_info), GFP_KERNEL);
> +	devinfo = kzalloc(sizeof(struct uas_dev_info), GFP_KERNEL);
>  	if (!devinfo)
>  		return -ENOMEM;
>  

Except for this hunk, which isn't an IU and doesn't go out on the wire.

Re: [PATCH] [USB] UAS: Use kzalloc instead of kmalloc

[Greg KH]

On Mon, Nov 08, 2010 at 06:21:31AM -0500, Matthew Wilcox wrote:
> On Wed, Oct 27, 2010 at 01:22:22PM -0700, Luben Tuikov wrote:
> > "Be conservative in what you send, liberal in what
> > you accept." -- In the spirit of this adage, don't
> > send Command IUs with randomly filled in data in
> > the reserved fields. (Yes, this shows up on the
> > wire.)
> 
> Applied, with a better changelog entry ...

Applied to where?  Shouldn't these patches go into my tree that feeds
into linux-next?

We really don't want driver-specific trees floating around anywhere,
please push them to me if possible.

thanks,

greg k-h

Re: [PATCH] [USB] UAS: Use kzalloc instead of kmalloc

[Luben Tuikov]

--- On Mon, 11/8/10, Matthew Wilcox <willy@linux.intel.com> wrote:
> On Wed, Oct 27, 2010 at 01:22:22PM
> -0700, Luben Tuikov wrote:
> > "Be conservative in what you send, liberal in what
> > you accept." -- In the spirit of this adage, don't
> > send Command IUs with randomly filled in data in
> > the reserved fields. (Yes, this shows up on the
> > wire.)
> 
> Applied, with a better changelog entry ...

"Better"? Where did you apply it? Your willy/uas.git doesn't show it (updated 3 months ago), neither do Greg's.

BTW, is it customary to change the change log?  What did you change? Do you mind sharing?

> > @@ -660,7 +660,7 @@ static int uas_probe(struct
> usb_interface *intf, const struct usb_device_id *id)
> >         
>     return -ENODEV;
> >      }
> >  
> > -    devinfo = kmalloc(sizeof(struct
> uas_dev_info), GFP_KERNEL);
> > +    devinfo = kzalloc(sizeof(struct
> uas_dev_info), GFP_KERNEL);
> >      if (!devinfo)
> >          return
> -ENOMEM;
> >  
> 
> Except for this hunk, which isn't an IU and doesn't go out
> on the wire.

Lol, no of course it doesn't, silly!

So help us understand: You've preserved all changes from kmalloc->kzalloc and left a single kmalloc alone. And your reason is that "*This* one doesn't go out on the wire?"

Wouldn't if have been more consistent (and harmless) to have changed all of them, just as the patch did?

Re: [PATCH] [USB] UAS: Use kzalloc instead of kmalloc

[Luben Tuikov]

Patch retracted for this still-born driver. Instead please use superior uasp.c which was recently submitted.

--- On Wed, 10/27/10, Luben Tuikov <ltuikov@yahoo.com> wrote:

> From: Luben Tuikov <ltuikov@yahoo.com>
> Subject: [PATCH] [USB] UAS: Use kzalloc instead of kmalloc
> To: "Greg KH" <greg@kroah.com>, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org, willy@linux.intel.com
> Date: Wednesday, October 27, 2010, 1:22 PM
> "Be conservative in what you send,
> liberal in what
> you accept." -- In the spirit of this adage, don't
> send Command IUs with randomly filled in data in
> the reserved fields. (Yes, this shows up on the
> wire.)
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  drivers/usb/storage/uas.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/storage/uas.c
> b/drivers/usb/storage/uas.c
> index 2054b1e..17e1820 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -294,7 +294,7 @@ static struct urb
> *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t
> gfp,
>      if (!urb)
>          goto out;
>  
> -    iu = kmalloc(sizeof(*iu), gfp);
> +    iu = kzalloc(sizeof(*iu), gfp);
>      if (!iu)
>          goto free;
>  
> @@ -325,7 +325,7 @@ static struct urb
> *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp,
>      if (len < 0)
>          len = 0;
>      len = ALIGN(len, 4);
> -    iu = kmalloc(sizeof(*iu) + len, gfp);
> +    iu = kzalloc(sizeof(*iu) + len, gfp);
>      if (!iu)
>          goto free;
>  
> @@ -660,7 +660,7 @@ static int uas_probe(struct
> usb_interface *intf, const struct usb_device_id *id)
>             
> return -ENODEV;
>      }
>  
> -    devinfo = kmalloc(sizeof(struct
> uas_dev_info), GFP_KERNEL);
> +    devinfo = kzalloc(sizeof(struct
> uas_dev_info), GFP_KERNEL);
>      if (!devinfo)
>          return -ENOMEM;
>  
> -- 
> 1.7.0.1
> 
> 

Re: [PATCH] [USB] UAS: Use kzalloc instead of kmalloc

[Luben Tuikov]

--- On Mon, 11/8/10, Luben Tuikov <ltuikov@yahoo.com> wrote:
> --- On Mon, 11/8/10, Luben Tuikov
> <ltuikov@yahoo.com>
> wrote:
> > --- On Mon, 11/8/10, Matthew Wilcox
> > <willy@linux.intel.com>
> > wrote:
> > > On Wed, Oct 27, 2010 at 01:22:22PM
> > > -0700, Luben Tuikov wrote:
> > > > "Be conservative in what you send, liberal
> in
> > what
> > > > you accept." -- In the spirit of this
> adage,
> > don't
> > > > send Command IUs with randomly filled in
> data in
> > > > the reserved fields. (Yes, this shows up on
> the
> > > > wire.)
> > > 
> > > Applied, with a better changelog entry ...
> > 
> > "Better"? Where did you apply it? Your willy/uas.git
> > doesn't show it (updated 3 months ago), neither do
> Greg's.
> > 
> > BTW, is it customary to change the change log?  What
> > did you change? Do you mind sharing?
> 
> Matthew, could you reply-all here with your new, changed
> and
> modified change log?
> 
> I'd like you to be accountable to what and how you've
> changed the changelog HERE and not in a git three somewhere
> and have an open comparison to what your new change log is
> and what is says.
> 
> Mine, quoted above:
>   a) mentions an adage that's been around for 30 years
> at least,
>      in the UNIX/net field to which we
> adhere.
>   b) mentions (only!) the Command IU of making out on
> the wire
>      with stale system data of the
> memory used for it.
> 
> Could you please reconsider your professional integrity
> and
> submit the patch as is with the original log?

Matthew,

I did NOT Signed-off-by: with my name and email to a patch where you've changed both the change log and the diff.

Now, you have two options here:

a) commit the modified by you patches, {change log and diff}, AS YOUR OWN patches, removing the Signed-off-by: me line, OR

b) commit the patch, {change log and diff}, as I've posted it, thus preserving the Signed-off-by: me line.

A Signed-off-by: line is a digital signature, and we cannot have that under text (the change log) that that person didn't write or agree to.

I don't mind either a) or b), but if a) then we need to include this practice of changing the patch {change log and diff} by the maintainer and removing the Signed-off-by: line of the author of the patch in Documentation/development-process/ so that contributors to the kernel are aware of this practice.

> 
> > > > @@ -660,7 +660,7 @@ static int
> uas_probe(struct
> > > usb_interface *intf, const struct usb_device_id
> *id)
> > > >         
> > >     return -ENODEV;
> > > >      }
> > > >  
> > > > -    devinfo = kmalloc(sizeof(struct
> > > uas_dev_info), GFP_KERNEL);
> > > > +    devinfo = kzalloc(sizeof(struct
> > > uas_dev_info), GFP_KERNEL);
> > > >      if (!devinfo)
> > > >          return
> > > -ENOMEM;
> > > >  
> > > 
> > > Except for this hunk, which isn't an IU and
> doesn't go
> > out
> > > on the wire.
> > 
> > Lol, no of course it doesn't, silly!
> 
> And notice that /my/ change log doesn't claim that 
> uas_dev_info makes it out the wire. It only mentions that
> the Command IU makes it out the wire with stale data.
> 
> > 
> > So help us understand: You've preserved all changes
> from
> > kmalloc->kzalloc and left a single kmalloc alone.
> And
> > your reason is that "*This* one doesn't go out on the
> > wire?"
> > 
> > Wouldn't if have been more consistent (and harmless)
> to
> > have changed all of them, just as the patch did?
> > 
> >
> 

Re: [PATCH] [USB] UAS: Use kzalloc instead of kmalloc

[Luben Tuikov]

--- On Mon, 11/8/10, Luben Tuikov <ltuikov@yahoo.com> wrote:
> --- On Mon, 11/8/10, Matthew Wilcox
> <willy@linux.intel.com>
> wrote:
> > On Wed, Oct 27, 2010 at 01:22:22PM
> > -0700, Luben Tuikov wrote:
> > > "Be conservative in what you send, liberal in
> what
> > > you accept." -- In the spirit of this adage,
> don't
> > > send Command IUs with randomly filled in data in
> > > the reserved fields. (Yes, this shows up on the
> > > wire.)
> > 
> > Applied, with a better changelog entry ...
> 
> "Better"? Where did you apply it? Your willy/uas.git
> doesn't show it (updated 3 months ago), neither do Greg's.
> 
> BTW, is it customary to change the change log?  What
> did you change? Do you mind sharing?

Matthew, could you reply-all here with your new, changed and
modified change log?

I'd like you to be accountable to what and how you've changed the changelog HERE and not in a git three somewhere and have an open comparison to what your new change log is and what is says.

Mine, quoted above:
  a) mentions an adage that's been around for 30 years at least,
     in the UNIX/net field to which we adhere.
  b) mentions (only!) the Command IU of making out on the wire
     with stale system data of the memory used for it.

Could you please reconsider your professional integrity and
submit the patch as is with the original log?

> > > @@ -660,7 +660,7 @@ static int uas_probe(struct
> > usb_interface *intf, const struct usb_device_id *id)
> > >         
> >     return -ENODEV;
> > >      }
> > >  
> > > -    devinfo = kmalloc(sizeof(struct
> > uas_dev_info), GFP_KERNEL);
> > > +    devinfo = kzalloc(sizeof(struct
> > uas_dev_info), GFP_KERNEL);
> > >      if (!devinfo)
> > >          return
> > -ENOMEM;
> > >  
> > 
> > Except for this hunk, which isn't an IU and doesn't go
> out
> > on the wire.
> 
> Lol, no of course it doesn't, silly!

And notice that /my/ change log doesn't claim that 
uas_dev_info makes it out the wire. It only mentions that
the Command IU makes it out the wire with stale data.

> 
> So help us understand: You've preserved all changes from
> kmalloc->kzalloc and left a single kmalloc alone. And
> your reason is that "*This* one doesn't go out on the
> wire?"
> 
> Wouldn't if have been more consistent (and harmless) to
> have changed all of them, just as the patch did?
> 
>