diff mbox series

ASoC: SOF: debug: Fix a potential issue on string buffer termination

Message ID 20210208072257.73900-1-hui.wang@canonical.com
State Superseded
Headers show
Series ASoC: SOF: debug: Fix a potential issue on string buffer termination | expand

Commit Message

Hui Wang Feb. 8, 2021, 7:22 a.m. UTC
The function simple_write_to_buffer() doesn't add string termination
at the end of buf, we need to add it on our own if calling that
function to write the size of count chars to buf. This change refers
to the function tokenize_input() in debug.c and the function
sof_dfsentry_trace_filter_write() in trace.c.

We didn't find this potential issue in the past because sometimes we
are very lucky, we kzalloc the size of count buf, the kernel not only
returns a buf with buf[0 ... (count - 1)] = 0 but buf[count] = 0, with
this luck, this issue will not be exposed.

Fixes: 091c12e1f50c ("ASoC: SOF: debug: add new debugfs entries for IPC flood test")
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 sound/soc/sof/debug.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Kai Vehmanen Feb. 8, 2021, 9:32 a.m. UTC | #1
Hi,

On Mon, 8 Feb 2021, Hui Wang wrote:

> The function simple_write_to_buffer() doesn't add string termination
> at the end of buf, we need to add it on our own if calling that
> function to write the size of count chars to buf. This change refers
> to the function tokenize_input() in debug.c and the function
> sof_dfsentry_trace_filter_write() in trace.c.
[...]
> --- a/sound/soc/sof/debug.c
> +++ b/sound/soc/sof/debug.c
> @@ -352,9 +352,10 @@ static ssize_t sof_dfsentry_write(struct file *file, const char __user *buffer,
>  	char *string;
>  	int ret;
>  
> -	string = kzalloc(count, GFP_KERNEL);
> +	string = kzalloc(count+1, GFP_KERNEL);

ouch, good catch, thanks! We have this correct in soc/sof/trace.c, but not 
here. To keep up with kernel style, maybe:

+	string = kzalloc(count + 1, GFP_KERNEL);

>  	if (!string)
>  		return -ENOMEM;
> +	string[count] = '\0';

kzalloc() returns zeros, so no need for this.

Br, Kai
Hui Wang Feb. 8, 2021, 10:14 a.m. UTC | #2
On 2/8/21 5:32 PM, Kai Vehmanen wrote:
> Hi,
>
> On Mon, 8 Feb 2021, Hui Wang wrote:
>
>> The function simple_write_to_buffer() doesn't add string termination
>> at the end of buf, we need to add it on our own if calling that
>> function to write the size of count chars to buf. This change refers
>> to the function tokenize_input() in debug.c and the function
>> sof_dfsentry_trace_filter_write() in trace.c.
> [...]
>> --- a/sound/soc/sof/debug.c
>> +++ b/sound/soc/sof/debug.c
>> @@ -352,9 +352,10 @@ static ssize_t sof_dfsentry_write(struct file *file, const char __user *buffer,
>>   	char *string;
>>   	int ret;
>>   
>> -	string = kzalloc(count, GFP_KERNEL);
>> +	string = kzalloc(count+1, GFP_KERNEL);
> ouch, good catch, thanks! We have this correct in soc/sof/trace.c, but not
> here. To keep up with kernel style, maybe:
>
> +	string = kzalloc(count + 1, GFP_KERNEL);
>
>>   	if (!string)
>>   		return -ENOMEM;
>> +	string[count] = '\0';
> kzalloc() returns zeros, so no need for this.

Right, Other places use kmalloc(), here kzalloc() doesn't need to set 0. 
Will drop it in the v2.

Thanks.

Hui.

>
> Br, Kai
diff mbox series

Patch

diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c
index 30213a1beaaa..edd4893119dd 100644
--- a/sound/soc/sof/debug.c
+++ b/sound/soc/sof/debug.c
@@ -352,9 +352,10 @@  static ssize_t sof_dfsentry_write(struct file *file, const char __user *buffer,
 	char *string;
 	int ret;
 
-	string = kzalloc(count, GFP_KERNEL);
+	string = kzalloc(count+1, GFP_KERNEL);
 	if (!string)
 		return -ENOMEM;
+	string[count] = '\0';
 
 	size = simple_write_to_buffer(string, count, ppos, buffer, count);
 	ret = size;