diff mbox series

Subject: [PATCH] fs/doio:Use the snprintf function to prevent buffer overflow

Message ID 3620f7c2.2af5.18768f7ce86.Coremail.crawler2015@163.com
State Changes Requested
Headers show
Series Subject: [PATCH] fs/doio:Use the snprintf function to prevent buffer overflow | expand

Commit Message

zenghao April 10, 2023, 2:21 a.m. UTC 
From 044a942863dc0ac84701bc14d78f4d7fc0bb8dcb Mon Sep 17 00:00:00 2001
From: Hao Zeng <zenghao@kylinos.cn>
Date: Mon, 10 Apr 2023 09:17:34 +0800
Subject: [PATCH] fs/doio:Use the snprintf function to prevent buffer overflow
 Use the snprintf function instead of sprintf in the write_log.c file
 Signed-off-by: Hao Zeng <zenghao@kylinos.cn>


---
 testcases/kernel/fs/doio/write_log.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)


-- 
2.37.2

Comments

Cyril Hrubis April 21, 2023, 11:25 a.m. UTC | #1 
Hi!
> From 044a942863dc0ac84701bc14d78f4d7fc0bb8dcb Mon Sep 17 00:00:00 2001
> From: Hao Zeng <zenghao@kylinos.cn>
> Date: Mon, 10 Apr 2023 09:17:34 +0800
> Subject: [PATCH] fs/doio:Use the snprintf function to prevent buffer overflow
>  Use the snprintf function instead of sprintf in the write_log.c file
>  Signed-off-by: Hao Zeng <zenghao@kylinos.cn>

The actual change looks good, however the patch is corrupted, looks like
your email client replaced spaces 0x20 with unicode non-breakable spaces
0xc2a0.

See: https://www.kernel.org/doc/html/v4.10/process/email-clients.html

> ---
>  testcases/kernel/fs/doio/write_log.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> 
> diff --git a/testcases/kernel/fs/doio/write_log.c b/testcases/kernel/fs/doio/write_log.c
> index e8ef9c7cb..86297b01d 100644
> --- a/testcases/kernel/fs/doio/write_log.c
> +++ b/testcases/kernel/fs/doio/write_log.c
> @@ -129,7 +129,7 @@ int wlog_open(struct wlog_file *wfile, int trunc, int mode)
>  	umask(omask);
>  
>  	if (wfile->w_afd == -1) {
> -		sprintf(Wlog_Error_String,
> +		snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
                                           ^
					   There should be space after
					   comma.
zenghao April 23, 2023, 3:04 a.m. UTC | #2 
Hi Cyril, Thanks very much for your review!

I have changed my email address to zenghao@kylinos.cn and provided V2.
Thank you again for the correction, I will learn and correct it


Best regards
-- Hao
At 2023-04-21 19:25:45, "Cyril Hrubis" <chrubis@suse.cz> wrote:
>Hi!
>> From 044a942863dc0ac84701bc14d78f4d7fc0bb8dcb Mon Sep 17 00:00:00 2001
>> From: Hao Zeng <zenghao@kylinos.cn>
>> Date: Mon, 10 Apr 2023 09:17:34 +0800
>> Subject: [PATCH] fs/doio:Use the snprintf function to prevent buffer overflow
>>  Use the snprintf function instead of sprintf in the write_log.c file
>>  Signed-off-by: Hao Zeng <zenghao@kylinos.cn>
>
>The actual change looks good, however the patch is corrupted, looks like
>your email client replaced spaces 0x20 with unicode non-breakable spaces
>0xc2a0.
>
>See: https://www.kernel.org/doc/html/v4.10/process/email-clients.html
>
>> ---
>>  testcases/kernel/fs/doio/write_log.c | 20 ++++++++++----------
>>  1 file changed, 10 insertions(+), 10 deletions(-)
>> 
>> 
>> diff --git a/testcases/kernel/fs/doio/write_log.c b/testcases/kernel/fs/doio/write_log.c
>> index e8ef9c7cb..86297b01d 100644
>> --- a/testcases/kernel/fs/doio/write_log.c
>> +++ b/testcases/kernel/fs/doio/write_log.c
>> @@ -129,7 +129,7 @@ int wlog_open(struct wlog_file *wfile, int trunc, int mode)
>>  	umask(omask);
>>  
>>  	if (wfile->w_afd == -1) {
>> -		sprintf(Wlog_Error_String,
>> +		snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
>                                           ^
>					   There should be space after
>					   comma.
>
>-- 
>Cyril Hrubis
>chrubis@suse.cz
diff mbox series

Patch

diff --git a/testcases/kernel/fs/doio/write_log.c b/testcases/kernel/fs/doio/write_log.c
index e8ef9c7cb..86297b01d 100644
--- a/testcases/kernel/fs/doio/write_log.c
+++ b/testcases/kernel/fs/doio/write_log.c
@@ -129,7 +129,7 @@  int wlog_open(struct wlog_file *wfile, int trunc, int mode)
 	umask(omask);
 
 	if (wfile->w_afd == -1) {
-		sprintf(Wlog_Error_String,
+		snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
 			"Could not open write_log - open(%s, %#o, %#o) failed:  %s\n",
 			wfile->w_file, oflags, mode, strerror(errno));
 		return -1;
@@ -141,7 +141,7 @@  int wlog_open(struct wlog_file *wfile, int trunc, int mode)
 
 	oflags = O_RDWR;
 	if ((wfile->w_rfd = open(wfile->w_file, oflags)) == -1) {
-		sprintf(Wlog_Error_String,
+		snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
 			"Could not open write log - open(%s, %#o) failed:  %s\n",
 			wfile->w_file, oflags, strerror(errno));
 		close(wfile->w_afd);
@@ -218,14 +218,14 @@  int wlog_record_write(struct wlog_file *wfile, struct wlog_rec *wrec,
 		reclen += 2;
 
 		if (write(wfile->w_afd, wbuf, reclen) == -1) {
-			sprintf(Wlog_Error_String,
+			snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
 				"Could not write log - write(%s, %s, %d) failed:  %s\n",
 				wfile->w_file, wbuf, reclen, strerror(errno));
 			return -1;
 		} else {
 			offset = lseek(wfile->w_afd, 0, SEEK_CUR) - reclen;
 			if (offset == -1) {
-				sprintf(Wlog_Error_String,
+				snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
 					"Could not reposition file pointer - lseek(%s, 0, SEEK_CUR) failed:  %s\n",
 					wfile->w_file, strerror(errno));
 				return -1;
@@ -233,13 +233,13 @@  int wlog_record_write(struct wlog_file *wfile, struct wlog_rec *wrec,
 		}
 	} else {
 		if ((lseek(wfile->w_rfd, offset, SEEK_SET)) == -1) {
-			sprintf(Wlog_Error_String,
+			snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
 				"Could not reposition file pointer - lseek(%s, %ld, SEEK_SET) failed:  %s\n",
 				wfile->w_file, offset, strerror(errno));
 			return -1;
 		} else {
 			if ((write(wfile->w_rfd, wbuf, reclen)) == -1) {
-				sprintf(Wlog_Error_String,
+				snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
 					"Could not write log - write(%s, %s, %d) failed:  %s\n",
 					wfile->w_file, wbuf, reclen,
 					strerror(errno));
@@ -274,14 +274,14 @@  int wlog_scan_backward(struct wlog_file *wfile, int nrecs,
 	 */
 
 	if ((lseek(fd, 0, SEEK_END)) == -1) {
-		sprintf(Wlog_Error_String,
+		snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
 			"Could not reposition file pointer - lseek(%s, 0, SEEK_END) failed:  %s\n",
 			wfile->w_file, strerror(errno));
 		return -1;
 	}
 	offset = lseek(fd, 0, SEEK_CUR);
 	if ((offset == -1)) {
-		sprintf(Wlog_Error_String,
+		snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
 			"Could not reposition file pointer - lseek(%s, 0, SEEK_CUR) failed:  %s\n",
 			wfile->w_file, strerror(errno));
 		return -1;
@@ -309,7 +309,7 @@  int wlog_scan_backward(struct wlog_file *wfile, int nrecs,
 		 * Move to the proper file offset, and read into buf
 		 */
 		if ((lseek(fd, offset, SEEK_SET)) == -1) {
-			sprintf(Wlog_Error_String,
+			snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
 				"Could not reposition file pointer - lseek(%s, %d, SEEK_SET) failed:  %s\n",
 				wfile->w_file, offset, strerror(errno));
 			return -1;
@@ -318,7 +318,7 @@  int wlog_scan_backward(struct wlog_file *wfile, int nrecs,
 		nbytes = read(fd, bufstart, bufend - bufstart - leftover);
 
 		if (nbytes == -1) {
-			sprintf(Wlog_Error_String,
+			snprintf(Wlog_Error_String,sizeof(Wlog_Error_String),
 				"Could not read history file at offset %d - read(%d, %p, %d) failed:  %s\n",
 				offset, fd, bufstart,
 				(int)(bufend - bufstart - leftover),