Patchwork [Open-FCoE] fcoeadm: Fix possible buffer overflows

login
register
mail settings
Submitter Johannes Thumshirn
Date June 15, 2016, 2:47 p.m.
Message ID <20160615144741.23447-1-jthumshirn@suse.de>
Download mbox | patch
Permalink /patch/320/
State Accepted
Headers show

Comments

Johannes Thumshirn - June 15, 2016, 2:47 p.m.
Fix 3 possible buffer overflows when strncat()ing strings together.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fcoeadm_display.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Laurence Oberman - June 15, 2016, 2:56 p.m.
----- Original Message -----
> From: "Johannes Thumshirn" <jthumshirn@suse.de>
> To: "Vasu Dev" <vasu.dev@linux.intel.com>, fcoe-devel@open-fcoe.org
> Sent: Wednesday, June 15, 2016 10:47:41 AM
> Subject: [Open-FCoE] [PATCH] fcoeadm: Fix possible buffer overflows
> 
> Fix 3 possible buffer overflows when strncat()ing strings together.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  fcoeadm_display.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fcoeadm_display.c b/fcoeadm_display.c
> index 16ccb6b..7b95aa4 100644
> --- a/fcoeadm_display.c
> +++ b/fcoeadm_display.c
> @@ -267,13 +267,13 @@ static void show_full_lun_info(unsigned int hba,
> unsigned int port,
>  	if (!port_attrs)
>  		goto free_rport;
>  
> -	strncat(path, "/device/", sizeof(path));
> +	strncat(path, "/device/", sizeof(path) - strlen(path) - 1);
>  
>  	sa_sys_read_line(path, "rev", rev, sizeof(rev));
>  	sa_sys_read_line(path, "model", model, sizeof(model));
>  	sa_sys_read_line(path, "vendor", vendor, sizeof(vendor));
>  
> -	strncat(path, "block", sizeof(path));
> +	strncat(path, "block", sizeof(path) - strlen(path) - 1);
>  
>  	dir = opendir(path);
>  	if (!dir)
> @@ -349,7 +349,7 @@ static void show_short_lun_info(unsigned int hba,
> unsigned int port,
>  	sa_sys_read_line(path, "model", model, sizeof(model));
>  	sa_sys_read_line(path, "vendor", vendor, sizeof(vendor));
>  
> -	strncat(path, "block", sizeof(path));
> +	strncat(path, "block", sizeof(path) - strlen(path) - 1);
>  
>  	dir = opendir(path);
>  	if (!dir)
> --
> 2.8.4
> 
> _______________________________________________
> fcoe-devel mailing list
> fcoe-devel@open-fcoe.org
> http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel
> 

Looks correct to me, offset -1 to allow for string termination.
Reviewed-by Laurence Oberman <loberman@redhat.com>
Vasu Dev - June 16, 2016, 5:31 p.m.
On Wed, 2016-06-15 at 16:47 +0200, Johannes Thumshirn wrote:
> Fix 3 possible buffer overflows when strncat()ing strings together.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  fcoeadm_display.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fcoeadm_display.c b/fcoeadm_display.c
> index 16ccb6b..7b95aa4 100644
> --- a/fcoeadm_display.c
> +++ b/fcoeadm_display.c
> @@ -267,13 +267,13 @@ static void show_full_lun_info(unsigned int
> hba, unsigned int port,
>  	if (!port_attrs)
>  		goto free_rport;
>  
> -	strncat(path, "/device/", sizeof(path));
> +	strncat(path, "/device/", sizeof(path) - strlen(path) - 1);
>  
>  	sa_sys_read_line(path, "rev", rev, sizeof(rev));
>  	sa_sys_read_line(path, "model", model, sizeof(model));
>  	sa_sys_read_line(path, "vendor", vendor, sizeof(vendor));
>  
> -	strncat(path, "block", sizeof(path));
> +	strncat(path, "block", sizeof(path) - strlen(path) - 1);
>  
>  	dir = opendir(path);
>  	if (!dir)
> @@ -349,7 +349,7 @@ static void show_short_lun_info(unsigned int hba,
> unsigned int port,
>  	sa_sys_read_line(path, "model", model, sizeof(model));
>  	sa_sys_read_line(path, "vendor", vendor, sizeof(vendor));
>  
> -	strncat(path, "block", sizeof(path));
> +	strncat(path, "block", sizeof(path) - strlen(path) - 1);
>  
>  	dir = opendir(path);
>  	if (!dir)

Looks good, I'll apply this and it may be last patch as I'm moving away
from maintaining this project.

//Vasu
Johannes Thumshirn - June 17, 2016, 7:06 a.m.
On Thu, Jun 16, 2016 at 10:31:55AM -0700, Vasu Dev wrote:
> On Wed, 2016-06-15 at 16:47 +0200, Johannes Thumshirn wrote:
> > Fix 3 possible buffer overflows when strncat()ing strings together.
> > 
> > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> > ---
> >  fcoeadm_display.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fcoeadm_display.c b/fcoeadm_display.c
> > index 16ccb6b..7b95aa4 100644
> > --- a/fcoeadm_display.c
> > +++ b/fcoeadm_display.c
> > @@ -267,13 +267,13 @@ static void show_full_lun_info(unsigned int
> > hba, unsigned int port,
> >  	if (!port_attrs)
> >  		goto free_rport;
> >  
> > -	strncat(path, "/device/", sizeof(path));
> > +	strncat(path, "/device/", sizeof(path) - strlen(path) - 1);
> >  
> >  	sa_sys_read_line(path, "rev", rev, sizeof(rev));
> >  	sa_sys_read_line(path, "model", model, sizeof(model));
> >  	sa_sys_read_line(path, "vendor", vendor, sizeof(vendor));
> >  
> > -	strncat(path, "block", sizeof(path));
> > +	strncat(path, "block", sizeof(path) - strlen(path) - 1);
> >  
> >  	dir = opendir(path);
> >  	if (!dir)
> > @@ -349,7 +349,7 @@ static void show_short_lun_info(unsigned int hba,
> > unsigned int port,
> >  	sa_sys_read_line(path, "model", model, sizeof(model));
> >  	sa_sys_read_line(path, "vendor", vendor, sizeof(vendor));
> >  
> > -	strncat(path, "block", sizeof(path));
> > +	strncat(path, "block", sizeof(path) - strlen(path) - 1);
> >  
> >  	dir = opendir(path);
> >  	if (!dir)
> 
> Looks good, I'll apply this and it may be last patch as I'm moving away
> from maintaining this project.

Yeah, heared the rumors. Sad that it's true but thanks for the Job you did.

It was nice working with you.

	Johannes

Patch

diff --git a/fcoeadm_display.c b/fcoeadm_display.c
index 16ccb6b..7b95aa4 100644
--- a/fcoeadm_display.c
+++ b/fcoeadm_display.c
@@ -267,13 +267,13 @@  static void show_full_lun_info(unsigned int hba, unsigned int port,
 	if (!port_attrs)
 		goto free_rport;
 
-	strncat(path, "/device/", sizeof(path));
+	strncat(path, "/device/", sizeof(path) - strlen(path) - 1);
 
 	sa_sys_read_line(path, "rev", rev, sizeof(rev));
 	sa_sys_read_line(path, "model", model, sizeof(model));
 	sa_sys_read_line(path, "vendor", vendor, sizeof(vendor));
 
-	strncat(path, "block", sizeof(path));
+	strncat(path, "block", sizeof(path) - strlen(path) - 1);
 
 	dir = opendir(path);
 	if (!dir)
@@ -349,7 +349,7 @@  static void show_short_lun_info(unsigned int hba, unsigned int port,
 	sa_sys_read_line(path, "model", model, sizeof(model));
 	sa_sys_read_line(path, "vendor", vendor, sizeof(vendor));
 
-	strncat(path, "block", sizeof(path));
+	strncat(path, "block", sizeof(path) - strlen(path) - 1);
 
 	dir = opendir(path);
 	if (!dir)