linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc/vio: Fix modalias_show return values
@ 2013-10-17  3:53 Ben Hutchings
  2013-10-17  5:49 ` Benjamin Herrenschmidt
  2013-10-17 11:50 ` Prarit Bhargava
  0 siblings, 2 replies; 3+ messages in thread
From: Ben Hutchings @ 2013-10-17  3:53 UTC (permalink / raw)
  To: Prarit Bhargava, Benjamin Herrenschmidt; +Cc: linuxppc-dev, stable

[-- Attachment #1: Type: text/plain, Size: 1084 bytes --]

Commit e82b89a6f19bae73fb064d1b3dd91fcefbb478f4 introduces a trivial
local denial of service.

> --- a/arch/powerpc/kernel/vio.c
> +++ b/arch/powerpc/kernel/vio.c
> @@ -1351,11 +1351,15 @@ static ssize_t modalias_show(struct devi
>  	const char *cp;
>  
>  	dn = dev->of_node;
> -	if (!dn)
> -		return -ENODEV;
> +	if (!dn) {
> +		strcat(buf, "\n");

Every read from the same sysfs file handle uses the same buffer, which
gets zero-initialised just once.  So if I open the file, read it and
seek back to 0 repeatedly, I can make modalias_show() write arbitrary
numbers of newlines into *and beyond* that page-sized buffer.

Obviously strcat() should be strcpy().

Ben.

> +		return strlen(buf);
> +	}
>  	cp = of_get_property(dn, "compatible", NULL);
> -	if (!cp)
> -		return -ENODEV;
> +	if (!cp) {
> +		strcat(buf, "\n");
> +		return strlen(buf);
> +	}
>  
>  	return sprintf(buf, "vio:T%sS%s\n", vio_dev->type, cp);
>  }

-- 
Ben Hutchings
Horngren's Observation:
                   Among economists, the real world is often a special case.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] powerpc/vio: Fix modalias_show return values
  2013-10-17  3:53 [PATCH] powerpc/vio: Fix modalias_show return values Ben Hutchings
@ 2013-10-17  5:49 ` Benjamin Herrenschmidt
  2013-10-17 11:50 ` Prarit Bhargava
  1 sibling, 0 replies; 3+ messages in thread
From: Benjamin Herrenschmidt @ 2013-10-17  5:49 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Prarit Bhargava, linuxppc-dev, stable

On Thu, 2013-10-17 at 04:53 +0100, Ben Hutchings wrote:
> Commit e82b89a6f19bae73fb064d1b3dd91fcefbb478f4 introduces a trivial
> local denial of service.

Oops. Prarit, please send a fix asap ! I'm travelling right now.

Thanks !
Ben.

> > --- a/arch/powerpc/kernel/vio.c
> > +++ b/arch/powerpc/kernel/vio.c
> > @@ -1351,11 +1351,15 @@ static ssize_t modalias_show(struct devi
> >  	const char *cp;
> >  
> >  	dn = dev->of_node;
> > -	if (!dn)
> > -		return -ENODEV;
> > +	if (!dn) {
> > +		strcat(buf, "\n");
> 
> Every read from the same sysfs file handle uses the same buffer, which
> gets zero-initialised just once.  So if I open the file, read it and
> seek back to 0 repeatedly, I can make modalias_show() write arbitrary
> numbers of newlines into *and beyond* that page-sized buffer.
> 
> Obviously strcat() should be strcpy().
> 
> Ben.
> 
> > +		return strlen(buf);
> > +	}
> >  	cp = of_get_property(dn, "compatible", NULL);
> > -	if (!cp)
> > -		return -ENODEV;
> > +	if (!cp) {
> > +		strcat(buf, "\n");
> > +		return strlen(buf);
> > +	}
> >  
> >  	return sprintf(buf, "vio:T%sS%s\n", vio_dev->type, cp);
> >  }
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] powerpc/vio: Fix modalias_show return values
  2013-10-17  3:53 [PATCH] powerpc/vio: Fix modalias_show return values Ben Hutchings
  2013-10-17  5:49 ` Benjamin Herrenschmidt
@ 2013-10-17 11:50 ` Prarit Bhargava
  1 sibling, 0 replies; 3+ messages in thread
From: Prarit Bhargava @ 2013-10-17 11:50 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: linuxppc-dev, stable



On 10/16/2013 11:53 PM, Ben Hutchings wrote:
> Commit e82b89a6f19bae73fb064d1b3dd91fcefbb478f4 introduces a trivial
> local denial of service.
> 
>> --- a/arch/powerpc/kernel/vio.c
>> +++ b/arch/powerpc/kernel/vio.c
>> @@ -1351,11 +1351,15 @@ static ssize_t modalias_show(struct devi
>>  	const char *cp;
>>  
>>  	dn = dev->of_node;
>> -	if (!dn)
>> -		return -ENODEV;
>> +	if (!dn) {
>> +		strcat(buf, "\n");
> 
> Every read from the same sysfs file handle uses the same buffer, which
> gets zero-initialised just once.  So if I open the file, read it and
> seek back to 0 repeatedly, I can make modalias_show() write arbitrary
> numbers of newlines into *and beyond* that page-sized buffer.
> 
> Obviously strcat() should be strcpy().
> 

D'oh!  Of course -- I wasn't thinking clearly about that.  I'll send out a new
patch.

P.

> Ben.
> 
>> +		return strlen(buf);
>> +	}
>>  	cp = of_get_property(dn, "compatible", NULL);
>> -	if (!cp)
>> -		return -ENODEV;
>> +	if (!cp) {
>> +		strcat(buf, "\n");
>> +		return strlen(buf);
>> +	}
>>  
>>  	return sprintf(buf, "vio:T%sS%s\n", vio_dev->type, cp);
>>  }
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-10-17 11:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-17  3:53 [PATCH] powerpc/vio: Fix modalias_show return values Ben Hutchings
2013-10-17  5:49 ` Benjamin Herrenschmidt
2013-10-17 11:50 ` Prarit Bhargava

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).