Re: [Freeipmi-devel] [PATCH] Using DMI firmware in sysfs if exists

2017-06-29 Thread Albert Chu
It seems important, so I'll put a release on my todo queue.

Al

On Wed, 2017-06-28 at 11:16 +0800, Ike Panhc wrote:
> Hi Albert,
> 
> Thanks for feedback.
> 
> You are the one to make the call. and I believe it
> is good to have a new release so every distro can
> have this fix especially they have arm64 release.
> 
> --
> Ike Panhc
> 
> 
> On 06/28/2017 07:42 AM, Albert Chu wrote:
> > LGTM, thanks!  This seems important/serious enough that it would affect
> > current users.  Do you think a new release is warranted?
> > 
> > Al
> > 
> > On Tue, 2017-06-27 at 16:07 +0800, Ike Panhc wrote:
> >> On arm64 system scanning /dev/mem for SMBIOS tables might fail and
> >> stopped with SIGBUS. Kernel version > 4.2 provides DMI tables in
> >> /sys/firmware/dmi/tables and it is much safer for ipmi-locate to
> >> read. If it exists, reading from it instead of /dev/mem fix
> >> ipmi-locate crash on several arm64 system.
> >>
> >> Unfortunately mmap() does not work on DMI tables so also fallback to
> >> read if mmap() failed.
> >>
> >> Signed-off-by: Ike Panhc 
> >> ---
> >>  libfreeipmi/locate/ipmi-locate-dmidecode.c | 28 
> >> +++-
> >>  1 file changed, 23 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/libfreeipmi/locate/ipmi-locate-dmidecode.c 
> >> b/libfreeipmi/locate/ipmi-locate-dmidecode.c
> >> index 8a89fc0..db1c473 100644
> >> --- a/libfreeipmi/locate/ipmi-locate-dmidecode.c
> >> +++ b/libfreeipmi/locate/ipmi-locate-dmidecode.c
> >> @@ -115,7 +115,6 @@ struct dmi_header
> >>fipmiu16 handle;
> >>  };
> >>  
> >> -#ifndef HAVE_MMAP
> >>  static int
> >>  _myread (ipmi_locate_ctx_t ctx,
> >>   int fd,
> >> @@ -155,7 +154,6 @@ _myread (ipmi_locate_ctx_t ctx,
> >>  
> >>return (0);
> >>  }
> >> -#endif
> >>  
> >>  static int
> >>  _checksum (const fipmiu8 *buf, size_t len)
> >> @@ -233,14 +231,16 @@ _mem_chunk (ipmi_locate_ctx_t ctx,
> >> base - mmoffset)) == MAP_FAILED)
> >>  {
> >>LOCATE_ERRNO_TO_LOCATE_ERRNUM (ctx, errno);
> >> -  goto cleanup;
> >> +  goto try_read;
> >>  }
> >>  
> >>memcpy (p, (fipmiu8 *) mmp + mmoffset, len);
> >>rv = p;
> >>/* ignore potential error, just return result */
> >>munmap (mmp, mmoffset + len);
> >> -#else /* HAVE_MMAP */
> >> +  goto cleanup;
> >> + try_read:
> >> +#endif /* HAVE_MMAP */
> >>  
> >>if (lseek (fd, base, SEEK_SET) < 0)
> >>  {
> >> @@ -252,7 +252,6 @@ _mem_chunk (ipmi_locate_ctx_t ctx,
> >>  goto cleanup;
> >>  
> >>rv = p;
> >> -#endif /* HAVE_MMAP */
> >>  
> >>   cleanup:
> >>/* ignore potential error, cleanup path */
> >> @@ -457,6 +456,8 @@ ipmi_locate_dmidecode_get_device_info 
> >> (ipmi_locate_ctx_t ctx,
> >>char linebuf[64];
> >>fipmiu8 *buf = NULL;
> >>int rv = -1;
> >> +  static char const sys_fw_dmi_tables[] = "/sys/firmware/dmi/tables/DMI";
> >> +  struct stat st;
> >>  
> >>if (!ctx || ctx->magic != IPMI_LOCATE_CTX_MAGIC)
> >>  {
> >> @@ -471,6 +472,23 @@ ipmi_locate_dmidecode_get_device_info 
> >> (ipmi_locate_ctx_t ctx,
> >>  }
> >>  
> >>memset (_info, '\0', sizeof (struct ipmi_locate_info));
> >> +
> >> +  /* if DMI firmware exist, use it and do not dig into /dev/mem */
> >> +  if (!(stat (sys_fw_dmi_tables, )))
> >> +{
> >> +  rv = _dmi_table (ctx,
> >> +0,
> >> +st.st_size,
> >> +st.st_size / 4,
> >> +0,
> >> +sys_fw_dmi_tables,
> >> +type,
> >> +_info);
> >> +  if (!(rv))
> >> +memcpy (info, _info, sizeof (struct ipmi_locate_info));
> >> +  return (rv);
> >> +}
> >> +
> >>/*
> >> * Linux up to 2.6.6-rc2: /proc/efi/systab
> >> * Linux 2.6.6-rc3 and up: /sys/firmware/efi/systab
> > 
> 

-- 
Albert Chu
ch...@llnl.gov
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory



___
Freeipmi-devel mailing list
Freeipmi-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/freeipmi-devel


Re: [Freeipmi-devel] [PATCH] Using DMI firmware in sysfs if exists

2017-06-27 Thread Ike Panhc
Hi Albert,

Thanks for feedback.

You are the one to make the call. and I believe it
is good to have a new release so every distro can
have this fix especially they have arm64 release.

--
Ike Panhc


On 06/28/2017 07:42 AM, Albert Chu wrote:
> LGTM, thanks!  This seems important/serious enough that it would affect
> current users.  Do you think a new release is warranted?
> 
> Al
> 
> On Tue, 2017-06-27 at 16:07 +0800, Ike Panhc wrote:
>> On arm64 system scanning /dev/mem for SMBIOS tables might fail and
>> stopped with SIGBUS. Kernel version > 4.2 provides DMI tables in
>> /sys/firmware/dmi/tables and it is much safer for ipmi-locate to
>> read. If it exists, reading from it instead of /dev/mem fix
>> ipmi-locate crash on several arm64 system.
>>
>> Unfortunately mmap() does not work on DMI tables so also fallback to
>> read if mmap() failed.
>>
>> Signed-off-by: Ike Panhc 
>> ---
>>  libfreeipmi/locate/ipmi-locate-dmidecode.c | 28 +++-
>>  1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/libfreeipmi/locate/ipmi-locate-dmidecode.c 
>> b/libfreeipmi/locate/ipmi-locate-dmidecode.c
>> index 8a89fc0..db1c473 100644
>> --- a/libfreeipmi/locate/ipmi-locate-dmidecode.c
>> +++ b/libfreeipmi/locate/ipmi-locate-dmidecode.c
>> @@ -115,7 +115,6 @@ struct dmi_header
>>fipmiu16 handle;
>>  };
>>  
>> -#ifndef HAVE_MMAP
>>  static int
>>  _myread (ipmi_locate_ctx_t ctx,
>>   int fd,
>> @@ -155,7 +154,6 @@ _myread (ipmi_locate_ctx_t ctx,
>>  
>>return (0);
>>  }
>> -#endif
>>  
>>  static int
>>  _checksum (const fipmiu8 *buf, size_t len)
>> @@ -233,14 +231,16 @@ _mem_chunk (ipmi_locate_ctx_t ctx,
>> base - mmoffset)) == MAP_FAILED)
>>  {
>>LOCATE_ERRNO_TO_LOCATE_ERRNUM (ctx, errno);
>> -  goto cleanup;
>> +  goto try_read;
>>  }
>>  
>>memcpy (p, (fipmiu8 *) mmp + mmoffset, len);
>>rv = p;
>>/* ignore potential error, just return result */
>>munmap (mmp, mmoffset + len);
>> -#else /* HAVE_MMAP */
>> +  goto cleanup;
>> + try_read:
>> +#endif /* HAVE_MMAP */
>>  
>>if (lseek (fd, base, SEEK_SET) < 0)
>>  {
>> @@ -252,7 +252,6 @@ _mem_chunk (ipmi_locate_ctx_t ctx,
>>  goto cleanup;
>>  
>>rv = p;
>> -#endif /* HAVE_MMAP */
>>  
>>   cleanup:
>>/* ignore potential error, cleanup path */
>> @@ -457,6 +456,8 @@ ipmi_locate_dmidecode_get_device_info (ipmi_locate_ctx_t 
>> ctx,
>>char linebuf[64];
>>fipmiu8 *buf = NULL;
>>int rv = -1;
>> +  static char const sys_fw_dmi_tables[] = "/sys/firmware/dmi/tables/DMI";
>> +  struct stat st;
>>  
>>if (!ctx || ctx->magic != IPMI_LOCATE_CTX_MAGIC)
>>  {
>> @@ -471,6 +472,23 @@ ipmi_locate_dmidecode_get_device_info 
>> (ipmi_locate_ctx_t ctx,
>>  }
>>  
>>memset (_info, '\0', sizeof (struct ipmi_locate_info));
>> +
>> +  /* if DMI firmware exist, use it and do not dig into /dev/mem */
>> +  if (!(stat (sys_fw_dmi_tables, )))
>> +{
>> +  rv = _dmi_table (ctx,
>> +0,
>> +st.st_size,
>> +st.st_size / 4,
>> +0,
>> +sys_fw_dmi_tables,
>> +type,
>> +_info);
>> +  if (!(rv))
>> +memcpy (info, _info, sizeof (struct ipmi_locate_info));
>> +  return (rv);
>> +}
>> +
>>/*
>> * Linux up to 2.6.6-rc2: /proc/efi/systab
>> * Linux 2.6.6-rc3 and up: /sys/firmware/efi/systab
> 


___
Freeipmi-devel mailing list
Freeipmi-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/freeipmi-devel


Re: [Freeipmi-devel] [PATCH] Using DMI firmware in sysfs if exists

2017-06-27 Thread Albert Chu
LGTM, thanks!  This seems important/serious enough that it would affect
current users.  Do you think a new release is warranted?

Al

On Tue, 2017-06-27 at 16:07 +0800, Ike Panhc wrote:
> On arm64 system scanning /dev/mem for SMBIOS tables might fail and
> stopped with SIGBUS. Kernel version > 4.2 provides DMI tables in
> /sys/firmware/dmi/tables and it is much safer for ipmi-locate to
> read. If it exists, reading from it instead of /dev/mem fix
> ipmi-locate crash on several arm64 system.
> 
> Unfortunately mmap() does not work on DMI tables so also fallback to
> read if mmap() failed.
> 
> Signed-off-by: Ike Panhc 
> ---
>  libfreeipmi/locate/ipmi-locate-dmidecode.c | 28 +++-
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/libfreeipmi/locate/ipmi-locate-dmidecode.c 
> b/libfreeipmi/locate/ipmi-locate-dmidecode.c
> index 8a89fc0..db1c473 100644
> --- a/libfreeipmi/locate/ipmi-locate-dmidecode.c
> +++ b/libfreeipmi/locate/ipmi-locate-dmidecode.c
> @@ -115,7 +115,6 @@ struct dmi_header
>fipmiu16 handle;
>  };
>  
> -#ifndef HAVE_MMAP
>  static int
>  _myread (ipmi_locate_ctx_t ctx,
>   int fd,
> @@ -155,7 +154,6 @@ _myread (ipmi_locate_ctx_t ctx,
>  
>return (0);
>  }
> -#endif
>  
>  static int
>  _checksum (const fipmiu8 *buf, size_t len)
> @@ -233,14 +231,16 @@ _mem_chunk (ipmi_locate_ctx_t ctx,
> base - mmoffset)) == MAP_FAILED)
>  {
>LOCATE_ERRNO_TO_LOCATE_ERRNUM (ctx, errno);
> -  goto cleanup;
> +  goto try_read;
>  }
>  
>memcpy (p, (fipmiu8 *) mmp + mmoffset, len);
>rv = p;
>/* ignore potential error, just return result */
>munmap (mmp, mmoffset + len);
> -#else /* HAVE_MMAP */
> +  goto cleanup;
> + try_read:
> +#endif /* HAVE_MMAP */
>  
>if (lseek (fd, base, SEEK_SET) < 0)
>  {
> @@ -252,7 +252,6 @@ _mem_chunk (ipmi_locate_ctx_t ctx,
>  goto cleanup;
>  
>rv = p;
> -#endif /* HAVE_MMAP */
>  
>   cleanup:
>/* ignore potential error, cleanup path */
> @@ -457,6 +456,8 @@ ipmi_locate_dmidecode_get_device_info (ipmi_locate_ctx_t 
> ctx,
>char linebuf[64];
>fipmiu8 *buf = NULL;
>int rv = -1;
> +  static char const sys_fw_dmi_tables[] = "/sys/firmware/dmi/tables/DMI";
> +  struct stat st;
>  
>if (!ctx || ctx->magic != IPMI_LOCATE_CTX_MAGIC)
>  {
> @@ -471,6 +472,23 @@ ipmi_locate_dmidecode_get_device_info (ipmi_locate_ctx_t 
> ctx,
>  }
>  
>memset (_info, '\0', sizeof (struct ipmi_locate_info));
> +
> +  /* if DMI firmware exist, use it and do not dig into /dev/mem */
> +  if (!(stat (sys_fw_dmi_tables, )))
> +{
> +  rv = _dmi_table (ctx,
> +0,
> +st.st_size,
> +st.st_size / 4,
> +0,
> +sys_fw_dmi_tables,
> +type,
> +_info);
> +  if (!(rv))
> +memcpy (info, _info, sizeof (struct ipmi_locate_info));
> +  return (rv);
> +}
> +
>/*
> * Linux up to 2.6.6-rc2: /proc/efi/systab
> * Linux 2.6.6-rc3 and up: /sys/firmware/efi/systab

-- 
Albert Chu
ch...@llnl.gov
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory



___
Freeipmi-devel mailing list
Freeipmi-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/freeipmi-devel


[Freeipmi-devel] [PATCH] Using DMI firmware in sysfs if exists

2017-06-27 Thread Ike Panhc
On arm64 system scanning /dev/mem for SMBIOS tables might fail and
stopped with SIGBUS. Kernel version > 4.2 provides DMI tables in
/sys/firmware/dmi/tables and it is much safer for ipmi-locate to
read. If it exists, reading from it instead of /dev/mem fix
ipmi-locate crash on several arm64 system.

Unfortunately mmap() does not work on DMI tables so also fallback to
read if mmap() failed.

Signed-off-by: Ike Panhc 
---
 libfreeipmi/locate/ipmi-locate-dmidecode.c | 28 +++-
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/libfreeipmi/locate/ipmi-locate-dmidecode.c 
b/libfreeipmi/locate/ipmi-locate-dmidecode.c
index 8a89fc0..db1c473 100644
--- a/libfreeipmi/locate/ipmi-locate-dmidecode.c
+++ b/libfreeipmi/locate/ipmi-locate-dmidecode.c
@@ -115,7 +115,6 @@ struct dmi_header
   fipmiu16 handle;
 };
 
-#ifndef HAVE_MMAP
 static int
 _myread (ipmi_locate_ctx_t ctx,
  int fd,
@@ -155,7 +154,6 @@ _myread (ipmi_locate_ctx_t ctx,
 
   return (0);
 }
-#endif
 
 static int
 _checksum (const fipmiu8 *buf, size_t len)
@@ -233,14 +231,16 @@ _mem_chunk (ipmi_locate_ctx_t ctx,
base - mmoffset)) == MAP_FAILED)
 {
   LOCATE_ERRNO_TO_LOCATE_ERRNUM (ctx, errno);
-  goto cleanup;
+  goto try_read;
 }
 
   memcpy (p, (fipmiu8 *) mmp + mmoffset, len);
   rv = p;
   /* ignore potential error, just return result */
   munmap (mmp, mmoffset + len);
-#else /* HAVE_MMAP */
+  goto cleanup;
+ try_read:
+#endif /* HAVE_MMAP */
 
   if (lseek (fd, base, SEEK_SET) < 0)
 {
@@ -252,7 +252,6 @@ _mem_chunk (ipmi_locate_ctx_t ctx,
 goto cleanup;
 
   rv = p;
-#endif /* HAVE_MMAP */
 
  cleanup:
   /* ignore potential error, cleanup path */
@@ -457,6 +456,8 @@ ipmi_locate_dmidecode_get_device_info (ipmi_locate_ctx_t 
ctx,
   char linebuf[64];
   fipmiu8 *buf = NULL;
   int rv = -1;
+  static char const sys_fw_dmi_tables[] = "/sys/firmware/dmi/tables/DMI";
+  struct stat st;
 
   if (!ctx || ctx->magic != IPMI_LOCATE_CTX_MAGIC)
 {
@@ -471,6 +472,23 @@ ipmi_locate_dmidecode_get_device_info (ipmi_locate_ctx_t 
ctx,
 }
 
   memset (_info, '\0', sizeof (struct ipmi_locate_info));
+
+  /* if DMI firmware exist, use it and do not dig into /dev/mem */
+  if (!(stat (sys_fw_dmi_tables, )))
+{
+  rv = _dmi_table (ctx,
+0,
+st.st_size,
+st.st_size / 4,
+0,
+sys_fw_dmi_tables,
+type,
+_info);
+  if (!(rv))
+memcpy (info, _info, sizeof (struct ipmi_locate_info));
+  return (rv);
+}
+
   /*
* Linux up to 2.6.6-rc2: /proc/efi/systab
* Linux 2.6.6-rc3 and up: /sys/firmware/efi/systab
-- 
2.7.4


___
Freeipmi-devel mailing list
Freeipmi-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/freeipmi-devel