[ofa-general] Re: [PATCH 4/6] [DAPL v1] fix off-by-one with ia_name

[Patrick Marchand Latifi]

You're right.  Thanks for catching that.

-pat

On Thu, Feb 14, 2008 at 09:25:09AM -0800, Arlin Davis wrote:
> Patrick Marchand Latifi wrote:
> >Make sure we stay within bounds when manipulating the ia_name.
> >
> >Signed-off-by: Patrick Marchand Latifi <patrick.latifi at qlogic.com>
> >---
> >
> > dat/udat/udat.c |    6 ++----
> > 1 files changed, 2 insertions(+), 4 deletions(-)
> >
> >diff --git a/dat/udat/udat.c b/dat/udat/udat.c
> >index e458441..c57d421 100644
> >--- a/dat/udat/udat.c
> >+++ b/dat/udat/udat.c
> >@@ -181,7 +181,7 @@ dat_ia_openv (
> > 
> >     len = dat_os_strlen (name);
> > 
> >-    if ( DAT_NAME_MAX_LENGTH < len )
> >+    if ( DAT_NAME_MAX_LENGTH <= len )
> >     {
> > 	return DAT_ERROR (DAT_INVALID_PARAMETER, DAT_INVALID_ARG1);
> >     }
> >@@ -197,7 +197,6 @@ dat_ia_openv (
> >     }
> > 
> >     dat_os_strncpy (info.ia_name, name, len);
> >-    info.ia_name[len] = '\0';
> 
> Same as DAPL 2.0 patch.
> 
> strlen does not include terminating NULL byte and strncpy
> will copy no more then len. Revising patch, adding len+1
> to get NULL byte with strncpy.
> 
> Here is a new patch for DAPL v1.2:
> 
> Signed-off by: Arlin Davis <ardavis at ichips.intel.com>
> 
> diff --git a/dat/udat/udat.c b/dat/udat/udat.c
> index e458441..cac1b93 100644
> --- a/dat/udat/udat.c
> +++ b/dat/udat/udat.c
> @@ -181,7 +181,7 @@ dat_ia_openv (
> 
>      len = dat_os_strlen (name);
> 
> -    if ( DAT_NAME_MAX_LENGTH < len )
> +    if ( DAT_NAME_MAX_LENGTH <= len )
>      {
>         return DAT_ERROR (DAT_INVALID_PARAMETER, DAT_INVALID_ARG1);
>      }
> @@ -196,8 +196,7 @@ dat_ia_openv (
>         return DAT_ERROR (DAT_INVALID_STATE, 0);
>      }
> 
> -    dat_os_strncpy (info.ia_name, name, len);
> -    info.ia_name[len] = '\0';
> +    dat_os_strncpy (info.ia_name, name, len+1);
> 
>      info.dapl_version_major = dapl_major;
>      info.dapl_version_minor = dapl_minor;
> @@ -301,10 +300,9 @@ dat_ia_close (
> 
>         len = dat_os_strlen (ia_name);
> 
> -       dat_os_assert ( len <= DAT_NAME_MAX_LENGTH );
> +       dat_os_assert ( len < DAT_NAME_MAX_LENGTH );
> 
> -       dat_os_strncpy (info.ia_name, ia_name, len);
> -       info.ia_name[len] = '\0';
> +       dat_os_strncpy (info.ia_name, ia_name, len+1);
> 
>         info.dapl_version_major = provider_attr.dapl_version_major;
>         info.dapl_version_minor = provider_attr.dapl_version_minor;