sysfs_ops show vector: size of buffer not required?

sysfs_ops show vector: size of buffer not required?

[Himanshu Chauhan]

Hi All,

The sysfs_ops's show vector doesn't have a size of the
buffer given to the vector, while store on the other hand
has. What is the rationale behind it?

I see most of the implementations doing strcpy in the
show vectors. Ill behaved driver might overwrite the
given buffer when size is not known. Should this be addressed
by providing the buffer size along with the buffer pointer?

Thanks

Best Regards
Himanshu

Re: sysfs_ops show vector: size of buffer not required?

[Greg KH]

On Sun, Jan 24, 2010 at 01:11:18PM +0530, Himanshu Chauhan wrote:
> Hi All,
> 
> The sysfs_ops's show vector doesn't have a size of the
> buffer given to the vector, while store on the other hand
> has. What is the rationale behind it?

If you need to check the size, you are doing something wrong.

Seriously, that is the reason.  A sysfs file should be a single value,
which will never overflow the buffer.

> I see most of the implementations doing strcpy in the
> show vectors. Ill behaved driver might overwrite the
> given buffer when size is not known. Should this be addressed
> by providing the buffer size along with the buffer pointer?

Nope.

Again, a single value only, it easily fits into the buffer size.

thanks,

greg k-h

Re: sysfs_ops show vector: size of buffer not required?

[Himanshu Chauhan]

On Mon, Jan 25, 2010 at 08:36:46PM -0800, Greg KH wrote:
> On Sun, Jan 24, 2010 at 01:11:18PM +0530, Himanshu Chauhan wrote:
> > Hi All,
> > 
> > The sysfs_ops's show vector doesn't have a size of the
> > buffer given to the vector, while store on the other hand
> > has. What is the rationale behind it?
> 
> If you need to check the size, you are doing something wrong.
> 
> Seriously, that is the reason.  A sysfs file should be a single value,
> which will never overflow the buffer.
> 
I was talking in context of usb/ip's show_status. It writes a lot of data
into this buffer. Which seems to over flow the buffer. But anyways, I
will check if it can be reduced or at least be splitted into differnt
device attributes.

BTW, Greg, Did you take a look at other patches I had sent? Are are worth or
I need rework?

Regards
Himanshu

Re: sysfs_ops show vector: size of buffer not required?

[Greg KH]

On Tue, Jan 26, 2010 at 12:01:27PM +0530, Himanshu Chauhan wrote:
> On Mon, Jan 25, 2010 at 08:36:46PM -0800, Greg KH wrote:
> > On Sun, Jan 24, 2010 at 01:11:18PM +0530, Himanshu Chauhan wrote:
> > > Hi All,
> > > 
> > > The sysfs_ops's show vector doesn't have a size of the
> > > buffer given to the vector, while store on the other hand
> > > has. What is the rationale behind it?
> > 
> > If you need to check the size, you are doing something wrong.
> > 
> > Seriously, that is the reason.  A sysfs file should be a single value,
> > which will never overflow the buffer.
> > 
> I was talking in context of usb/ip's show_status. It writes a lot of data
> into this buffer.

Then it needs to be fixed.  Again, it must be, one value per file, that
is the sysfs rule.

> Which seems to over flow the buffer. But anyways, I will check if it
> can be reduced or at least be splitted into differnt device
> attributes.

That would be great.

> BTW, Greg, Did you take a look at other patches I had sent? Are are worth or
> I need rework?

They are in my "to-apply" queue that I will be flushing out in the next
few days.

thanks,

greg k-h

Re: sysfs_ops show vector: size of buffer not required?

[Himanshu Chauhan]

On Tue, Jan 26, 2010 at 07:29:24AM -0800, Greg KH wrote:
> On Tue, Jan 26, 2010 at 12:01:27PM +0530, Himanshu Chauhan wrote:
> > On Mon, Jan 25, 2010 at 08:36:46PM -0800, Greg KH wrote:
> > > On Sun, Jan 24, 2010 at 01:11:18PM +0530, Himanshu Chauhan wrote:
> > > > Hi All,
> > > > 
> > > > The sysfs_ops's show vector doesn't have a size of the
> > > > buffer given to the vector, while store on the other hand
> > > > has. What is the rationale behind it?
> > > 
> > > If you need to check the size, you are doing something wrong.
> > > 
> > > Seriously, that is the reason.  A sysfs file should be a single value,
> > > which will never overflow the buffer.
> > > 
> > I was talking in context of usb/ip's show_status. It writes a lot of data
> > into this buffer.
> 
> Then it needs to be fixed.  Again, it must be, one value per file, that
> is the sysfs rule.
> 
> > Which seems to over flow the buffer. But anyways, I will check if it
> > can be reduced or at least be splitted into differnt device
> > attributes.
> 
> That would be great.

Okay, I will do that.

> 
> > BTW, Greg, Did you take a look at other patches I had sent? Are are worth or
> > I need rework?
> 
> They are in my "to-apply" queue that I will be flushing out in the next
> few days.

Thanks Greg.

Regards
Himanshu