powerpc/vio: use strcpy in modalias_show

Submitted by Prarit Bhargava on Oct. 17, 2013, noon

Details

Message ID 1382011211-15272-1-git-send-email-prarit@redhat.com
State Accepted, archived
Commit 411cabf79e684171669ad29a0628c400b4431e95
Headers show

Commit Message

Prarit Bhargava Oct. 17, 2013, noon
Commit e82b89a6f19bae73fb064d1b3dd91fcefbb478f4 used strcat instead of
strcpy which can result in an overflow of newlines on the buffer.

Signed-off-by: Prarit Bhargava
Cc: benh@kernel.crashing.org
Cc: ben@decadent.org.uk
Cc: stable@vger.kernel.org
---
 arch/powerpc/kernel/vio.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

David Laight Oct. 17, 2013, 12:22 p.m.
> Commit e82b89a6f19bae73fb064d1b3dd91fcefbb478f4 used strcat instead of
> strcpy which can result in an overflow of newlines on the buffer.
...
> --- a/arch/powerpc/kernel/vio.c
> +++ b/arch/powerpc/kernel/vio.c
> @@ -1531,12 +1531,12 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute
> *attr,
> 
>  	dn = dev->of_node;
>  	if (!dn) {
> -		strcat(buf, "\n");
> +		strcpy(buf, "\n");
>  		return strlen(buf);
>  	}
>  	cp = of_get_property(dn, "compatible", NULL);
>  	if (!cp) {
> -		strcat(buf, "\n");
> +		strcpy(buf, "\n");
>  		return strlen(buf);
>  	}

Why not just:
		buf[0] = '\n';
		buf[1] = 0;
		return 1;

The assignment to buf[1] might not even be needed.

	David
Prarit Bhargava Oct. 17, 2013, 12:29 p.m.
On 10/17/2013 08:22 AM, David Laight wrote:
>> Commit e82b89a6f19bae73fb064d1b3dd91fcefbb478f4 used strcat instead of
>> strcpy which can result in an overflow of newlines on the buffer.
> ...
>> --- a/arch/powerpc/kernel/vio.c
>> +++ b/arch/powerpc/kernel/vio.c
>> @@ -1531,12 +1531,12 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute
>> *attr,
>>
>>  	dn = dev->of_node;
>>  	if (!dn) {
>> -		strcat(buf, "\n");
>> +		strcpy(buf, "\n");
>>  		return strlen(buf);
>>  	}
>>  	cp = of_get_property(dn, "compatible", NULL);
>>  	if (!cp) {
>> -		strcat(buf, "\n");
>> +		strcpy(buf, "\n");
>>  		return strlen(buf);
>>  	}
> 
> Why not just:
> 		buf[0] = '\n';
> 		buf[1] = 0;
> 		return 1;
> 
> The assignment to buf[1] might not even be needed.

Sure, I guess that'd work too.  But it really seems like 1/2 a dozen of one and
six of the other.  I'll defer to the preference of the maintainers to see what
they want.

P.

> 
> 	David
> 
> 
>

Patch hide | download patch | download mbox

diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index d38cc08..cb92d82 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -1531,12 +1531,12 @@  static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
 
 	dn = dev->of_node;
 	if (!dn) {
-		strcat(buf, "\n");
+		strcpy(buf, "\n");
 		return strlen(buf);
 	}
 	cp = of_get_property(dn, "compatible", NULL);
 	if (!cp) {
-		strcat(buf, "\n");
+		strcpy(buf, "\n");
 		return strlen(buf);
 	}