Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2013-01-17 Thread Greg KH
On Tue, Dec 18, 2012 at 12:38:03PM +, Ben Hutchings wrote:
> On Tue, 2012-12-18 at 03:06 -0500, Tomas Hozza wrote:
> > - Original Message -
> > > > This is just for sanity. The value PATH_MAX was chosen after
> > > > discussion
> > > > with K. Y. Srinivasan and Olaf Hering instead of some "magic"
> > > > number like
> > > > 256 or 512.
> > > 
> > > PATH_MAX is a magic name.
> > 
> > It is defined in "limits.h". I would welcome some more constructive
> > argumentation and critics.
> 
> It still bears no relation to any actual limit in the C library or Linux
> kernel.  So it's no more valid than the previous number.
> 
> In the current context we're enumerating /sys/class/net and we know that
> all the interface names in there are limited to IFNAMSIZ-1 = 15 (there
> is also potentially "bonding_masters").  The longest path name we need
> to use is definitely much shorter than even 256 bytes.
> 
> > > > > Using snprintf() is a good idea, but you need to check the return
> > > > > value and handle the truncation case somehow.
> > > > 
> > > > By using PATH_MAX sized buffer there is no need for handling the
> > > > truncation
> > > > case.
> > >  
> > > You are claiming two contradictory things: sprintf() may overrun the
> > > buffer, so we need the length check provided by snprintf(), but there
> > > is no need to check for truncation because we know the length is
> > > sufficient.
> > 
> > So what do you propose? How should it be solved?
> 
>   if (snprintf(dev_id, sizeof(dev_id), ...) >= sizeof(dev_id))
>   continue;
> 
> Possibly logging a warning.

I agree, I'm dropping this patch from my to-apply queue.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2013-01-17 Thread Greg KH
On Tue, Dec 18, 2012 at 12:38:03PM +, Ben Hutchings wrote:
 On Tue, 2012-12-18 at 03:06 -0500, Tomas Hozza wrote:
  - Original Message -
This is just for sanity. The value PATH_MAX was chosen after
discussion
with K. Y. Srinivasan and Olaf Hering instead of some magic
number like
256 or 512.
   
   PATH_MAX is a magic name.
  
  It is defined in limits.h. I would welcome some more constructive
  argumentation and critics.
 
 It still bears no relation to any actual limit in the C library or Linux
 kernel.  So it's no more valid than the previous number.
 
 In the current context we're enumerating /sys/class/net and we know that
 all the interface names in there are limited to IFNAMSIZ-1 = 15 (there
 is also potentially bonding_masters).  The longest path name we need
 to use is definitely much shorter than even 256 bytes.
 
 Using snprintf() is a good idea, but you need to check the return
 value and handle the truncation case somehow.

By using PATH_MAX sized buffer there is no need for handling the
truncation
case.

   You are claiming two contradictory things: sprintf() may overrun the
   buffer, so we need the length check provided by snprintf(), but there
   is no need to check for truncation because we know the length is
   sufficient.
  
  So what do you propose? How should it be solved?
 
   if (snprintf(dev_id, sizeof(dev_id), ...) = sizeof(dev_id))
   continue;
 
 Possibly logging a warning.

I agree, I'm dropping this patch from my to-apply queue.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-12-18 Thread Ben Hutchings
On Tue, 2012-12-18 at 03:06 -0500, Tomas Hozza wrote:
> - Original Message -
> > > This is just for sanity. The value PATH_MAX was chosen after
> > > discussion
> > > with K. Y. Srinivasan and Olaf Hering instead of some "magic"
> > > number like
> > > 256 or 512.
> > 
> > PATH_MAX is a magic name.
> 
> It is defined in "limits.h". I would welcome some more constructive
> argumentation and critics.

It still bears no relation to any actual limit in the C library or Linux
kernel.  So it's no more valid than the previous number.

In the current context we're enumerating /sys/class/net and we know that
all the interface names in there are limited to IFNAMSIZ-1 = 15 (there
is also potentially "bonding_masters").  The longest path name we need
to use is definitely much shorter than even 256 bytes.

> > > > Using snprintf() is a good idea, but you need to check the return
> > > > value and handle the truncation case somehow.
> > > 
> > > By using PATH_MAX sized buffer there is no need for handling the
> > > truncation
> > > case.
> >  
> > You are claiming two contradictory things: sprintf() may overrun the
> > buffer, so we need the length check provided by snprintf(), but there
> > is no need to check for truncation because we know the length is
> > sufficient.
> 
> So what do you propose? How should it be solved?

if (snprintf(dev_id, sizeof(dev_id), ...) >= sizeof(dev_id))
continue;

Possibly logging a warning.

Ben.

-- 
Ben Hutchings
Life is like a sewer:
what you get out of it depends on what you put into it.


signature.asc
Description: This is a digitally signed message part


Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-12-18 Thread Tomas Hozza
- Original Message -
> > This is just for sanity. The value PATH_MAX was chosen after
> > discussion
> > with K. Y. Srinivasan and Olaf Hering instead of some "magic"
> > number like
> > 256 or 512.
> 
> PATH_MAX is a magic name.

It is defined in "limits.h". I would welcome some more constructive
argumentation and critics.

> > > Using snprintf() is a good idea, but you need to check the return
> > > value and handle the truncation case somehow.
> > 
> > By using PATH_MAX sized buffer there is no need for handling the
> > truncation
> > case.
>  
> You are claiming two contradictory things: sprintf() may overrun the
> buffer, so we need the length check provided by snprintf(), but there
> is no need to check for truncation because we know the length is
> sufficient.

So what do you propose? How should it be solved?

Thank you.

Regards,
Tomas Hozza
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-12-18 Thread Tomas Hozza
- Original Message -
  This is just for sanity. The value PATH_MAX was chosen after
  discussion
  with K. Y. Srinivasan and Olaf Hering instead of some magic
  number like
  256 or 512.
 
 PATH_MAX is a magic name.

It is defined in limits.h. I would welcome some more constructive
argumentation and critics.

   Using snprintf() is a good idea, but you need to check the return
   value and handle the truncation case somehow.
  
  By using PATH_MAX sized buffer there is no need for handling the
  truncation
  case.
  
 You are claiming two contradictory things: sprintf() may overrun the
 buffer, so we need the length check provided by snprintf(), but there
 is no need to check for truncation because we know the length is
 sufficient.

So what do you propose? How should it be solved?

Thank you.

Regards,
Tomas Hozza
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-12-18 Thread Ben Hutchings
On Tue, 2012-12-18 at 03:06 -0500, Tomas Hozza wrote:
 - Original Message -
   This is just for sanity. The value PATH_MAX was chosen after
   discussion
   with K. Y. Srinivasan and Olaf Hering instead of some magic
   number like
   256 or 512.
  
  PATH_MAX is a magic name.
 
 It is defined in limits.h. I would welcome some more constructive
 argumentation and critics.

It still bears no relation to any actual limit in the C library or Linux
kernel.  So it's no more valid than the previous number.

In the current context we're enumerating /sys/class/net and we know that
all the interface names in there are limited to IFNAMSIZ-1 = 15 (there
is also potentially bonding_masters).  The longest path name we need
to use is definitely much shorter than even 256 bytes.

Using snprintf() is a good idea, but you need to check the return
value and handle the truncation case somehow.
   
   By using PATH_MAX sized buffer there is no need for handling the
   truncation
   case.
   
  You are claiming two contradictory things: sprintf() may overrun the
  buffer, so we need the length check provided by snprintf(), but there
  is no need to check for truncation because we know the length is
  sufficient.
 
 So what do you propose? How should it be solved?

if (snprintf(dev_id, sizeof(dev_id), ...) = sizeof(dev_id))
continue;

Possibly logging a warning.

Ben.

-- 
Ben Hutchings
Life is like a sewer:
what you get out of it depends on what you put into it.


signature.asc
Description: This is a digitally signed message part


Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-11-27 Thread Ben Hutchings
On Tue, Nov 27, 2012 at 03:28:25PM -0500, Tomas Hozza wrote:
> 
> 
> - Original Message -
> > On Tue, 2012-11-27 at 08:56 +0100, Tomas Hozza wrote:
> > > kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
> > > sized buffers which could be too small to store really long names.
> > >
> > > Buffer sizes have been changed to PATH_MAX, include "limits.h"
> > > where
> > > PATH_MAX is defined was added and length checks ware added via
> > > snprintf.
> > [...]
> > 
> > PATH_MAX has nothing to do with any actual kernel limit; it's no more
> > meaningful than the current value of 256.  Network interface names
> > are
> > limited to 15 characters, thus the current array is more than long
> > enough.  So I think this is entirely unnecessary.
> 
> This is just for sanity. The value PATH_MAX was chosen after discussion
> with K. Y. Srinivasan and Olaf Hering instead of some "magic" number like
> 256 or 512.

PATH_MAX is a magic name.

> > Using snprintf() is a good idea, but you need to check the return
> > value and handle the truncation case somehow.
> 
> By using PATH_MAX sized buffer there is no need for handling the truncation
> case.
 
You are claiming two contradictory things: sprintf() may overrun the
buffer, so we need the length check provided by snprintf(), but there
is no need to check for truncation because we know the length is
sufficient.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
  - Albert Camus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-11-27 Thread Tomas Hozza


- Original Message -
> On Tue, 2012-11-27 at 08:56 +0100, Tomas Hozza wrote:
> > kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
> > sized buffers which could be too small to store really long names.
> >
> > Buffer sizes have been changed to PATH_MAX, include "limits.h"
> > where
> > PATH_MAX is defined was added and length checks ware added via
> > snprintf.
> [...]
> 
> PATH_MAX has nothing to do with any actual kernel limit; it's no more
> meaningful than the current value of 256.  Network interface names
> are
> limited to 15 characters, thus the current array is more than long
> enough.  So I think this is entirely unnecessary.

This is just for sanity. The value PATH_MAX was chosen after discussion
with K. Y. Srinivasan and Olaf Hering instead of some "magic" number like
256 or 512.

> Using snprintf() is a good idea, but you need to check the return
> value and handle the truncation case somehow.

By using PATH_MAX sized buffer there is no need for handling the truncation
case.


Tomas Hozza
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-11-27 Thread Ben Hutchings
On Tue, 2012-11-27 at 08:56 +0100, Tomas Hozza wrote:
> kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
> sized buffers which could be too small to store really long names.
>
> Buffer sizes have been changed to PATH_MAX, include "limits.h" where
> PATH_MAX is defined was added and length checks ware added via snprintf.
[...]

PATH_MAX has nothing to do with any actual kernel limit; it's no more
meaningful than the current value of 256.  Network interface names are
limited to 15 characters, thus the current array is more than long
enough.  So I think this is entirely unnecessary.

Using snprintf() is a good idea, but you need to check the return value
and handle the truncation case somehow.

Ben.

-- 
Ben Hutchings
Never attribute to conspiracy what can adequately be explained by stupidity.


signature.asc
Description: This is a digitally signed message part


RE: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-11-27 Thread KY Srinivasan


> -Original Message-
> From: Tomas Hozza [mailto:tho...@redhat.com]
> Sent: Tuesday, November 27, 2012 2:57 AM
> To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; KY Srinivasan; b...@decadent.org.uk
> Cc: Tomas Hozza
> Subject: [PATCH 1/3] tools/hv: Fix for long file names from readdir
> 
> kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
> sized buffers which could be too small to store really long names.
> 
> Buffer sizes have been changed to PATH_MAX, include "limits.h" where
> PATH_MAX is defined was added and length checks ware added via snprintf.
> 
> Signed-off-by: Tomas Hozza 
Acked-by:  K. Y. Srinivasan 

> ---
>  tools/hv/hv_kvp_daemon.c | 26 +-
>  1 file changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index d25a469..90f1f07 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -44,6 +44,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  /*
>   * KVP protocol: The user mode component first registers with the
> @@ -592,26 +593,22 @@ static char *kvp_get_if_name(char *guid)
>   DIR *dir;
>   struct dirent *entry;
>   FILE*file;
> - char*p, *q, *x;
> + char*p, *x;
>   char*if_name = NULL;
>   charbuf[256];
>   char *kvp_net_dir = "/sys/class/net/";
> - char dev_id[256];
> + char dev_id[PATH_MAX];
> 
>   dir = opendir(kvp_net_dir);
>   if (dir == NULL)
>   return NULL;
> 
> - snprintf(dev_id, sizeof(dev_id), "%s", kvp_net_dir);
> - q = dev_id + strlen(kvp_net_dir);
> -
>   while ((entry = readdir(dir)) != NULL) {
>   /*
>* Set the state for the next pass.
>*/
> - *q = '\0';
> - strcat(dev_id, entry->d_name);
> - strcat(dev_id, "/device/device_id");
> + snprintf(dev_id, sizeof(dev_id), "%s%s/device/device_id",
> kvp_net_dir,
> + entry->d_name);
> 
>   file = fopen(dev_id, "r");
>   if (file == NULL)
> @@ -684,28 +681,23 @@ static char *kvp_mac_to_if_name(char *mac)
>   DIR *dir;
>   struct dirent *entry;
>   FILE*file;
> - char*p, *q, *x;
> + char*p, *x;
>   char*if_name = NULL;
>   charbuf[256];
>   char *kvp_net_dir = "/sys/class/net/";
> - char dev_id[256];
> + char dev_id[PATH_MAX];
>   int i;
> 
>   dir = opendir(kvp_net_dir);
>   if (dir == NULL)
>   return NULL;
> 
> - snprintf(dev_id, sizeof(dev_id), kvp_net_dir);
> - q = dev_id + strlen(kvp_net_dir);
> -
>   while ((entry = readdir(dir)) != NULL) {
>   /*
>* Set the state for the next pass.
>*/
> - *q = '\0';
> -
> - strcat(dev_id, entry->d_name);
> - strcat(dev_id, "/address");
> + snprintf(dev_id, sizeof(dev_id), "%s%s/address", kvp_net_dir,
> +entry->d_name);
> 
>   file = fopen(dev_id, "r");
>   if (file == NULL)
> --
> 1.7.11.7
> 
> 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-11-27 Thread KY Srinivasan


 -Original Message-
 From: Tomas Hozza [mailto:tho...@redhat.com]
 Sent: Tuesday, November 27, 2012 2:57 AM
 To: gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
 de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
 jasow...@redhat.com; KY Srinivasan; b...@decadent.org.uk
 Cc: Tomas Hozza
 Subject: [PATCH 1/3] tools/hv: Fix for long file names from readdir
 
 kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
 sized buffers which could be too small to store really long names.
 
 Buffer sizes have been changed to PATH_MAX, include limits.h where
 PATH_MAX is defined was added and length checks ware added via snprintf.
 
 Signed-off-by: Tomas Hozza tho...@redhat.com
Acked-by:  K. Y. Srinivasan k...@microsoft.com

 ---
  tools/hv/hv_kvp_daemon.c | 26 +-
  1 file changed, 9 insertions(+), 17 deletions(-)
 
 diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
 index d25a469..90f1f07 100644
 --- a/tools/hv/hv_kvp_daemon.c
 +++ b/tools/hv/hv_kvp_daemon.c
 @@ -44,6 +44,7 @@
  #include fcntl.h
  #include dirent.h
  #include net/if.h
 +#include limits.h
 
  /*
   * KVP protocol: The user mode component first registers with the
 @@ -592,26 +593,22 @@ static char *kvp_get_if_name(char *guid)
   DIR *dir;
   struct dirent *entry;
   FILE*file;
 - char*p, *q, *x;
 + char*p, *x;
   char*if_name = NULL;
   charbuf[256];
   char *kvp_net_dir = /sys/class/net/;
 - char dev_id[256];
 + char dev_id[PATH_MAX];
 
   dir = opendir(kvp_net_dir);
   if (dir == NULL)
   return NULL;
 
 - snprintf(dev_id, sizeof(dev_id), %s, kvp_net_dir);
 - q = dev_id + strlen(kvp_net_dir);
 -
   while ((entry = readdir(dir)) != NULL) {
   /*
* Set the state for the next pass.
*/
 - *q = '\0';
 - strcat(dev_id, entry-d_name);
 - strcat(dev_id, /device/device_id);
 + snprintf(dev_id, sizeof(dev_id), %s%s/device/device_id,
 kvp_net_dir,
 + entry-d_name);
 
   file = fopen(dev_id, r);
   if (file == NULL)
 @@ -684,28 +681,23 @@ static char *kvp_mac_to_if_name(char *mac)
   DIR *dir;
   struct dirent *entry;
   FILE*file;
 - char*p, *q, *x;
 + char*p, *x;
   char*if_name = NULL;
   charbuf[256];
   char *kvp_net_dir = /sys/class/net/;
 - char dev_id[256];
 + char dev_id[PATH_MAX];
   int i;
 
   dir = opendir(kvp_net_dir);
   if (dir == NULL)
   return NULL;
 
 - snprintf(dev_id, sizeof(dev_id), kvp_net_dir);
 - q = dev_id + strlen(kvp_net_dir);
 -
   while ((entry = readdir(dir)) != NULL) {
   /*
* Set the state for the next pass.
*/
 - *q = '\0';
 -
 - strcat(dev_id, entry-d_name);
 - strcat(dev_id, /address);
 + snprintf(dev_id, sizeof(dev_id), %s%s/address, kvp_net_dir,
 +entry-d_name);
 
   file = fopen(dev_id, r);
   if (file == NULL)
 --
 1.7.11.7
 
 



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-11-27 Thread Ben Hutchings
On Tue, 2012-11-27 at 08:56 +0100, Tomas Hozza wrote:
 kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
 sized buffers which could be too small to store really long names.

 Buffer sizes have been changed to PATH_MAX, include limits.h where
 PATH_MAX is defined was added and length checks ware added via snprintf.
[...]

PATH_MAX has nothing to do with any actual kernel limit; it's no more
meaningful than the current value of 256.  Network interface names are
limited to 15 characters, thus the current array is more than long
enough.  So I think this is entirely unnecessary.

Using snprintf() is a good idea, but you need to check the return value
and handle the truncation case somehow.

Ben.

-- 
Ben Hutchings
Never attribute to conspiracy what can adequately be explained by stupidity.


signature.asc
Description: This is a digitally signed message part


Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-11-27 Thread Tomas Hozza


- Original Message -
 On Tue, 2012-11-27 at 08:56 +0100, Tomas Hozza wrote:
  kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
  sized buffers which could be too small to store really long names.
 
  Buffer sizes have been changed to PATH_MAX, include limits.h
  where
  PATH_MAX is defined was added and length checks ware added via
  snprintf.
 [...]
 
 PATH_MAX has nothing to do with any actual kernel limit; it's no more
 meaningful than the current value of 256.  Network interface names
 are
 limited to 15 characters, thus the current array is more than long
 enough.  So I think this is entirely unnecessary.

This is just for sanity. The value PATH_MAX was chosen after discussion
with K. Y. Srinivasan and Olaf Hering instead of some magic number like
256 or 512.

 Using snprintf() is a good idea, but you need to check the return
 value and handle the truncation case somehow.

By using PATH_MAX sized buffer there is no need for handling the truncation
case.


Tomas Hozza
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-11-27 Thread Ben Hutchings
On Tue, Nov 27, 2012 at 03:28:25PM -0500, Tomas Hozza wrote:
 
 
 - Original Message -
  On Tue, 2012-11-27 at 08:56 +0100, Tomas Hozza wrote:
   kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
   sized buffers which could be too small to store really long names.
  
   Buffer sizes have been changed to PATH_MAX, include limits.h
   where
   PATH_MAX is defined was added and length checks ware added via
   snprintf.
  [...]
  
  PATH_MAX has nothing to do with any actual kernel limit; it's no more
  meaningful than the current value of 256.  Network interface names
  are
  limited to 15 characters, thus the current array is more than long
  enough.  So I think this is entirely unnecessary.
 
 This is just for sanity. The value PATH_MAX was chosen after discussion
 with K. Y. Srinivasan and Olaf Hering instead of some magic number like
 256 or 512.

PATH_MAX is a magic name.

  Using snprintf() is a good idea, but you need to check the return
  value and handle the truncation case somehow.
 
 By using PATH_MAX sized buffer there is no need for handling the truncation
 case.
 
You are claiming two contradictory things: sprintf() may overrun the
buffer, so we need the length check provided by snprintf(), but there
is no need to check for truncation because we know the length is
sufficient.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
  - Albert Camus
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-11-26 Thread Tomas Hozza
kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
sized buffers which could be too small to store really long names.

Buffer sizes have been changed to PATH_MAX, include "limits.h" where
PATH_MAX is defined was added and length checks ware added via snprintf.

Signed-off-by: Tomas Hozza 
---
 tools/hv/hv_kvp_daemon.c | 26 +-
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index d25a469..90f1f07 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -44,6 +44,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * KVP protocol: The user mode component first registers with the
@@ -592,26 +593,22 @@ static char *kvp_get_if_name(char *guid)
DIR *dir;
struct dirent *entry;
FILE*file;
-   char*p, *q, *x;
+   char*p, *x;
char*if_name = NULL;
charbuf[256];
char *kvp_net_dir = "/sys/class/net/";
-   char dev_id[256];
+   char dev_id[PATH_MAX];
 
dir = opendir(kvp_net_dir);
if (dir == NULL)
return NULL;
 
-   snprintf(dev_id, sizeof(dev_id), "%s", kvp_net_dir);
-   q = dev_id + strlen(kvp_net_dir);
-
while ((entry = readdir(dir)) != NULL) {
/*
 * Set the state for the next pass.
 */
-   *q = '\0';
-   strcat(dev_id, entry->d_name);
-   strcat(dev_id, "/device/device_id");
+   snprintf(dev_id, sizeof(dev_id), "%s%s/device/device_id", 
kvp_net_dir,
+   entry->d_name);
 
file = fopen(dev_id, "r");
if (file == NULL)
@@ -684,28 +681,23 @@ static char *kvp_mac_to_if_name(char *mac)
DIR *dir;
struct dirent *entry;
FILE*file;
-   char*p, *q, *x;
+   char*p, *x;
char*if_name = NULL;
charbuf[256];
char *kvp_net_dir = "/sys/class/net/";
-   char dev_id[256];
+   char dev_id[PATH_MAX];
int i;
 
dir = opendir(kvp_net_dir);
if (dir == NULL)
return NULL;
 
-   snprintf(dev_id, sizeof(dev_id), kvp_net_dir);
-   q = dev_id + strlen(kvp_net_dir);
-
while ((entry = readdir(dir)) != NULL) {
/*
 * Set the state for the next pass.
 */
-   *q = '\0';
-
-   strcat(dev_id, entry->d_name);
-   strcat(dev_id, "/address");
+   snprintf(dev_id, sizeof(dev_id), "%s%s/address", kvp_net_dir,
+entry->d_name);
 
file = fopen(dev_id, "r");
if (file == NULL)
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/3] tools/hv: Fix for long file names from readdir

2012-11-26 Thread Tomas Hozza
kvp_get_if_name and kvp_mac_to_if_name copy strings into statically
sized buffers which could be too small to store really long names.

Buffer sizes have been changed to PATH_MAX, include limits.h where
PATH_MAX is defined was added and length checks ware added via snprintf.

Signed-off-by: Tomas Hozza tho...@redhat.com
---
 tools/hv/hv_kvp_daemon.c | 26 +-
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index d25a469..90f1f07 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -44,6 +44,7 @@
 #include fcntl.h
 #include dirent.h
 #include net/if.h
+#include limits.h
 
 /*
  * KVP protocol: The user mode component first registers with the
@@ -592,26 +593,22 @@ static char *kvp_get_if_name(char *guid)
DIR *dir;
struct dirent *entry;
FILE*file;
-   char*p, *q, *x;
+   char*p, *x;
char*if_name = NULL;
charbuf[256];
char *kvp_net_dir = /sys/class/net/;
-   char dev_id[256];
+   char dev_id[PATH_MAX];
 
dir = opendir(kvp_net_dir);
if (dir == NULL)
return NULL;
 
-   snprintf(dev_id, sizeof(dev_id), %s, kvp_net_dir);
-   q = dev_id + strlen(kvp_net_dir);
-
while ((entry = readdir(dir)) != NULL) {
/*
 * Set the state for the next pass.
 */
-   *q = '\0';
-   strcat(dev_id, entry-d_name);
-   strcat(dev_id, /device/device_id);
+   snprintf(dev_id, sizeof(dev_id), %s%s/device/device_id, 
kvp_net_dir,
+   entry-d_name);
 
file = fopen(dev_id, r);
if (file == NULL)
@@ -684,28 +681,23 @@ static char *kvp_mac_to_if_name(char *mac)
DIR *dir;
struct dirent *entry;
FILE*file;
-   char*p, *q, *x;
+   char*p, *x;
char*if_name = NULL;
charbuf[256];
char *kvp_net_dir = /sys/class/net/;
-   char dev_id[256];
+   char dev_id[PATH_MAX];
int i;
 
dir = opendir(kvp_net_dir);
if (dir == NULL)
return NULL;
 
-   snprintf(dev_id, sizeof(dev_id), kvp_net_dir);
-   q = dev_id + strlen(kvp_net_dir);
-
while ((entry = readdir(dir)) != NULL) {
/*
 * Set the state for the next pass.
 */
-   *q = '\0';
-
-   strcat(dev_id, entry-d_name);
-   strcat(dev_id, /address);
+   snprintf(dev_id, sizeof(dev_id), %s%s/address, kvp_net_dir,
+entry-d_name);
 
file = fopen(dev_id, r);
if (file == NULL)
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/