diff mbox

[1/1] ibacm: define prov_lib_path as a char array

Message ID 1404325850-918-1-git-send-email-kaike.wan@intel.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Wan, Kaike July 2, 2014, 6:30 p.m. UTC 
From: Kaike Wan <kaike.wan@intel.com>

This patch fixes a segfault error when the option file defines the provider
lib path. The variable prov_lib_path should be a buffer (char array) instead
of a char pointer. This allows a string to be copied into it during option
parsing.

Signed-off-by: Kaike Wan <kaike.wan@intel.com>
---
 src/acm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Ira Weiny July 2, 2014, 8:22 p.m. UTC | #1 
> 
> diff --git a/src/acm.c b/src/acm.c
> index 8f147ef..fae9d0b 100644
> --- a/src/acm.c
> +++ b/src/acm.c
> @@ -193,7 +193,7 @@ static int log_level = 0;  static char lock_file[128] =
> "/var/run/ibacm.pid";  static short server_port = 6125;  static int
> support_ips_in_addr_cfg = 0; -static char *prov_lib_path = IBACM_LIB_PATH;
> +static char prov_lib_path[256] = IBACM_LIB_PATH;

256 is too short for a path.  Typical is 4096 but would be nice if it were dynamic.

Ira


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ira Weiny July 2, 2014, 8:25 p.m. UTC | #2 
> >
> > diff --git a/src/acm.c b/src/acm.c
> > index 8f147ef..fae9d0b 100644
> > --- a/src/acm.c
> > +++ b/src/acm.c
> > @@ -193,7 +193,7 @@ static int log_level = 0;  static char
> > lock_file[128] = "/var/run/ibacm.pid";  static short server_port =
> > 6125;  static int support_ips_in_addr_cfg = 0; -static char
> > *prov_lib_path = IBACM_LIB_PATH;
> > +static char prov_lib_path[256] = IBACM_LIB_PATH;
> 
> 256 is too short for a path.  Typical is 4096 but would be nice if it were
> dynamic.

Oh and do you need the length as a #define somewhere to prevent overflowing the buffer when reading from the config file?

Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hefty, Sean July 2, 2014, 8:39 p.m. UTC | #3 
> > > diff --git a/src/acm.c b/src/acm.c
> > > index 8f147ef..fae9d0b 100644
> > > --- a/src/acm.c
> > > +++ b/src/acm.c
> > > @@ -193,7 +193,7 @@ static int log_level = 0;  static char
> > > lock_file[128] = "/var/run/ibacm.pid";  static short server_port =
> > > 6125;  static int support_ips_in_addr_cfg = 0; -static char
> > > *prov_lib_path = IBACM_LIB_PATH;
> > > +static char prov_lib_path[256] = IBACM_LIB_PATH;
> >
> > 256 is too short for a path.  Typical is 4096 but would be nice if it
> were
> > dynamic.
> 
> Oh and do you need the length as a #define somewhere to prevent overflowing
> the buffer when reading from the config file?

The code only reads in values from the config file that are up to 256 bytes long.  Have you seen paths longer than 256 in practice? 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ira Weiny July 2, 2014, 8:44 p.m. UTC | #4 
> > > > diff --git a/src/acm.c b/src/acm.c index 8f147ef..fae9d0b 100644
> > > > --- a/src/acm.c
> > > > +++ b/src/acm.c
> > > > @@ -193,7 +193,7 @@ static int log_level = 0;  static char
> > > > lock_file[128] = "/var/run/ibacm.pid";  static short server_port =
> > > > 6125;  static int support_ips_in_addr_cfg = 0; -static char
> > > > *prov_lib_path = IBACM_LIB_PATH;
> > > > +static char prov_lib_path[256] = IBACM_LIB_PATH;
> > >
> > > 256 is too short for a path.  Typical is 4096 but would be nice if
> > > it
> > were
> > > dynamic.
> >
> > Oh and do you need the length as a #define somewhere to prevent
> > overflowing the buffer when reading from the config file?
> 
> The code only reads in values from the config file that are up to 256 bytes long.
> Have you seen paths longer than 256 in practice?

In an official install perhaps not.  But I build in my home directory all the time.  ;-)

Even if the config file parser is limiting to 256 I don't think it would be an issue to make this buffer larger.  We can expand the config file parsing later if needed.

Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/src/acm.c b/src/acm.c
index 8f147ef..fae9d0b 100644
--- a/src/acm.c
+++ b/src/acm.c
@@ -193,7 +193,7 @@  static int log_level = 0;
 static char lock_file[128] = "/var/run/ibacm.pid";
 static short server_port = 6125;
 static int support_ips_in_addr_cfg = 0;
-static char *prov_lib_path = IBACM_LIB_PATH;
+static char prov_lib_path[256] = IBACM_LIB_PATH;
 
 void acm_write(int level, const char *format, ...)
 {