Re: [Freeipmi-devel] [PATCH] Using DMI firmware in sysfs if exists
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
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
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
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