Re: [Openipmi-developer] [PATCH] ipmi_si: Potential array underflow in hotmod_handler()y
On Fri, Feb 22, 2019 at 10:55:30PM +0300, Dan Carpenter wrote:
> The "ival" variable needs to signed so that we don't read before the
> start of the str[] array. This would only happen the user passed in a
> module parameter that was just comprised of space characters.
That was quick, I just uploaded that to linux-next yesterday. Thanks
for this, yes you are right. Added to my queue.
-corey
>
> Fixes: e80444ae4fc3 ("ipmi_si: Switch hotmod to use a platform device")
> Signed-off-by: Dan Carpenter
> ---
> drivers/char/ipmi/ipmi_si_hotmod.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/ipmi/ipmi_si_hotmod.c
> b/drivers/char/ipmi/ipmi_si_hotmod.c
> index 1433055a9705..03140f6cdf6f 100644
> --- a/drivers/char/ipmi/ipmi_si_hotmod.c
> +++ b/drivers/char/ipmi/ipmi_si_hotmod.c
> @@ -187,7 +187,8 @@ static int hotmod_handler(const char *val, const struct
> kernel_param *kp)
> char *str = kstrdup(val, GFP_KERNEL), *curr, *next;
> int rv;
> struct ipmi_plat_data h;
> - unsigned int len, ival;
> + unsigned int len;
> + int ival;
>
> if (!str)
> return -ENOMEM;
> --
> 2.17.1
>
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
[Openipmi-developer] [PATCH] ipmi_si: Potential array underflow in hotmod_handler()
The "ival" variable needs to signed so that we don't read before the
start of the str[] array. This would only happen the user passed in a
module parameter that was just comprised of space characters.
Fixes: e80444ae4fc3 ("ipmi_si: Switch hotmod to use a platform device")
Signed-off-by: Dan Carpenter
---
drivers/char/ipmi/ipmi_si_hotmod.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/char/ipmi/ipmi_si_hotmod.c
b/drivers/char/ipmi/ipmi_si_hotmod.c
index 1433055a9705..03140f6cdf6f 100644
--- a/drivers/char/ipmi/ipmi_si_hotmod.c
+++ b/drivers/char/ipmi/ipmi_si_hotmod.c
@@ -187,7 +187,8 @@ static int hotmod_handler(const char *val, const struct
kernel_param *kp)
char *str = kstrdup(val, GFP_KERNEL), *curr, *next;
int rv;
struct ipmi_plat_data h;
- unsigned int len, ival;
+ unsigned int len;
+ int ival;
if (!str)
return -ENOMEM;
--
2.17.1
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH] ipmi_si: Fix crash when using hard-coded device
On Fri, Feb 22, 2019 at 05:11:49PM +0800, Yang Yingliang wrote: > Tested-by: Yang Yingliang Thanks for testing. This is queued for 5.1. I'd like to let the soak a bit in linux-next. It is tagged for a backport to stable releases. -corey > > [ 128.069528] ipmi_si: IPMI System Interface driver > [ 128.069556] ipmi_si dmi-ipmi-si.0: ipmi_platform: probing via SMBIOS > [ 128.069559] ipmi_platform: ipmi_si: SMBIOS: io 0xe4 regsize 1 spacing 1 > irq 0 > [ 128.069561] ipmi_si: Adding SMBIOS-specified bt state machine > [ 128.069657] ipmi_si hisi-lpc-ipmi.1.auto: ipmi_platform: probing via ACPI > [ 128.069694] ipmi_si hisi-lpc-ipmi.1.auto: ipmi_platform: [io > 0xffc0e3-0xffc0e7] regsize 1 spacing 1 irq 0 > [ 128.069696] ipmi_si hisi-lpc-ipmi.1.auto: Hard-coded device at this > address already exists > [ 128.069815] ipmi_si hardcode-ipmi-si.0: ipmi_platform: probing via > hardcoded > [ 128.069817] ipmi_platform: ipmi_si: hardcoded: io 0xffc0e3 regsize 1 > spacing 1 irq 0 > [ 128.069819] ipmi_si: Adding hardcoded-specified bt state machine > [ 128.069892] ipmi_si: Trying SMBIOS-specified bt state machine at i/o > address 0xe4, slave address 0x0, irq 0 > [ 128.069896] ipmi_si dmi-ipmi-si.0: Could not set up I/O space > [ 128.069900] ipmi_si: Trying hardcoded-specified bt state machine at i/o > address 0xffc0e3, slave address 0x0, irq 0 > [ 128.081782] ipmi_si hardcode-ipmi-si.0: bt cap response too short: 3 > [ 128.081784] ipmi_si hardcode-ipmi-si.0: using default values > [ 128.081786] ipmi_si hardcode-ipmi-si.0: req2rsp=5 secs retries=2 > [ 128.157855] ipmi_si hardcode-ipmi-si.0: IPMI message handler: Found new > BMC (man_id: 0x0007db, prod_id: 0x0001, dev_id: 0x01) > [ 128.441860] ipmi_si hardcode-ipmi-si.0: IPMI bt interface initialized > > On 2019/2/22 2:33, miny...@acm.org wrote: > > From: Corey Minyard > > > > When excuting a command like: > >modprobe ipmi_si ports=0xffc0e3 type=bt > > The system would get an oops. > > > > The trouble here is that ipmi_si_hardcode_find_bmc() is called before > > ipmi_si_platform_init(), but initialization of the hard-coded device > > creates an IPMI platform device, which won't be initialized yet. > > > > The real trouble is that hard-coded devices aren't created with > > any device, and the fixup is done later. So do it right, create the > > hard-coded devices as normal platform devices. > > > > This required adding some new resource types to the IPMI platform > > code for passing information required by the hard-coded device > > and adding some code to remove the hard-coded platform devices > > on module removal. > > > > To enforce the "hard-coded devices passed by the user take priority > > over firmware devices" rule, some special code was added to check > > and see if a hard-coded device already exists. > > > > Reported-by: Yang Yingliang > > Signed-off-by: Corey Minyard > > --- > > > > I believe this is the right fix. Can you test/review and send the > > results? > > > > Thanks, > > > > -corey > > > > drivers/char/ipmi/ipmi_si.h | 4 +- > > drivers/char/ipmi/ipmi_si_hardcode.c | 236 --- > > drivers/char/ipmi/ipmi_si_intf.c | 23 ++- > > drivers/char/ipmi/ipmi_si_platform.c | 25 ++- > > 4 files changed, 214 insertions(+), 74 deletions(-) > > > > diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h > > index 52f6152d1fcb..7ae52c17618e 100644 > > --- a/drivers/char/ipmi/ipmi_si.h > > +++ b/drivers/char/ipmi/ipmi_si.h > > @@ -25,7 +25,9 @@ void ipmi_irq_finish_setup(struct si_sm_io *io); > > int ipmi_si_remove_by_dev(struct device *dev); > > void ipmi_si_remove_by_data(int addr_space, enum si_type si_type, > > unsigned long addr); > > -int ipmi_si_hardcode_find_bmc(void); > > +void ipmi_hardcode_init(void); > > +void ipmi_si_hardcode_exit(void); > > +int ipmi_si_hardcode_match(int addr_type, unsigned long addr); > > void ipmi_si_platform_init(void); > > void ipmi_si_platform_shutdown(void); > > diff --git a/drivers/char/ipmi/ipmi_si_hardcode.c > > b/drivers/char/ipmi/ipmi_si_hardcode.c > > index 487642809c58..1e5783961b0d 100644 > > --- a/drivers/char/ipmi/ipmi_si_hardcode.c > > +++ b/drivers/char/ipmi/ipmi_si_hardcode.c > > @@ -3,6 +3,7 @@ > > #define pr_fmt(fmt) "ipmi_hardcode: " fmt > > #include > > +#include > > #include "ipmi_si.h" > > /* > > @@ -12,23 +13,22 @@ > > #define SI_MAX_PARMS 4 > > -static char *si_type[SI_MAX_PARMS]; > > #define MAX_SI_TYPE_STR 30 > > -static char si_type_str[MAX_SI_TYPE_STR]; > > +static char si_type_str[MAX_SI_TYPE_STR] __initdata; > > static unsigned long addrs[SI_MAX_PARMS]; > > static unsigned int num_addrs; > > static unsigned int ports[SI_MAX_PARMS]; > > static unsigned int num_ports; > > -static int irqs[SI_MAX_PARMS]; > > -static unsigned int num_irqs; > > -static int regspacings[SI_MAX_PARMS]; > > -static unsigned int
Re: [Openipmi-developer] [PATCH] ipmi_si: Fix crash when using hard-coded device
Tested-by: Yang Yingliang [ 128.069528] ipmi_si: IPMI System Interface driver [ 128.069556] ipmi_si dmi-ipmi-si.0: ipmi_platform: probing via SMBIOS [ 128.069559] ipmi_platform: ipmi_si: SMBIOS: io 0xe4 regsize 1 spacing 1 irq 0 [ 128.069561] ipmi_si: Adding SMBIOS-specified bt state machine [ 128.069657] ipmi_si hisi-lpc-ipmi.1.auto: ipmi_platform: probing via ACPI [ 128.069694] ipmi_si hisi-lpc-ipmi.1.auto: ipmi_platform: [io 0xffc0e3-0xffc0e7] regsize 1 spacing 1 irq 0 [ 128.069696] ipmi_si hisi-lpc-ipmi.1.auto: Hard-coded device at this address already exists [ 128.069815] ipmi_si hardcode-ipmi-si.0: ipmi_platform: probing via hardcoded [ 128.069817] ipmi_platform: ipmi_si: hardcoded: io 0xffc0e3 regsize 1 spacing 1 irq 0 [ 128.069819] ipmi_si: Adding hardcoded-specified bt state machine [ 128.069892] ipmi_si: Trying SMBIOS-specified bt state machine at i/o address 0xe4, slave address 0x0, irq 0 [ 128.069896] ipmi_si dmi-ipmi-si.0: Could not set up I/O space [ 128.069900] ipmi_si: Trying hardcoded-specified bt state machine at i/o address 0xffc0e3, slave address 0x0, irq 0 [ 128.081782] ipmi_si hardcode-ipmi-si.0: bt cap response too short: 3 [ 128.081784] ipmi_si hardcode-ipmi-si.0: using default values [ 128.081786] ipmi_si hardcode-ipmi-si.0: req2rsp=5 secs retries=2 [ 128.157855] ipmi_si hardcode-ipmi-si.0: IPMI message handler: Found new BMC (man_id: 0x0007db, prod_id: 0x0001, dev_id: 0x01) [ 128.441860] ipmi_si hardcode-ipmi-si.0: IPMI bt interface initialized On 2019/2/22 2:33, miny...@acm.org wrote: From: Corey Minyard When excuting a command like: modprobe ipmi_si ports=0xffc0e3 type=bt The system would get an oops. The trouble here is that ipmi_si_hardcode_find_bmc() is called before ipmi_si_platform_init(), but initialization of the hard-coded device creates an IPMI platform device, which won't be initialized yet. The real trouble is that hard-coded devices aren't created with any device, and the fixup is done later. So do it right, create the hard-coded devices as normal platform devices. This required adding some new resource types to the IPMI platform code for passing information required by the hard-coded device and adding some code to remove the hard-coded platform devices on module removal. To enforce the "hard-coded devices passed by the user take priority over firmware devices" rule, some special code was added to check and see if a hard-coded device already exists. Reported-by: Yang Yingliang Signed-off-by: Corey Minyard --- I believe this is the right fix. Can you test/review and send the results? Thanks, -corey drivers/char/ipmi/ipmi_si.h | 4 +- drivers/char/ipmi/ipmi_si_hardcode.c | 236 --- drivers/char/ipmi/ipmi_si_intf.c | 23 ++- drivers/char/ipmi/ipmi_si_platform.c | 25 ++- 4 files changed, 214 insertions(+), 74 deletions(-) diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h index 52f6152d1fcb..7ae52c17618e 100644 --- a/drivers/char/ipmi/ipmi_si.h +++ b/drivers/char/ipmi/ipmi_si.h @@ -25,7 +25,9 @@ void ipmi_irq_finish_setup(struct si_sm_io *io); int ipmi_si_remove_by_dev(struct device *dev); void ipmi_si_remove_by_data(int addr_space, enum si_type si_type, unsigned long addr); -int ipmi_si_hardcode_find_bmc(void); +void ipmi_hardcode_init(void); +void ipmi_si_hardcode_exit(void); +int ipmi_si_hardcode_match(int addr_type, unsigned long addr); void ipmi_si_platform_init(void); void ipmi_si_platform_shutdown(void); diff --git a/drivers/char/ipmi/ipmi_si_hardcode.c b/drivers/char/ipmi/ipmi_si_hardcode.c index 487642809c58..1e5783961b0d 100644 --- a/drivers/char/ipmi/ipmi_si_hardcode.c +++ b/drivers/char/ipmi/ipmi_si_hardcode.c @@ -3,6 +3,7 @@ #define pr_fmt(fmt) "ipmi_hardcode: " fmt #include +#include #include "ipmi_si.h" /* @@ -12,23 +13,22 @@ #define SI_MAX_PARMS 4 -static char *si_type[SI_MAX_PARMS]; #define MAX_SI_TYPE_STR 30 -static char si_type_str[MAX_SI_TYPE_STR]; +static char si_type_str[MAX_SI_TYPE_STR] __initdata; static unsigned long addrs[SI_MAX_PARMS]; static unsigned int num_addrs; static unsigned int ports[SI_MAX_PARMS]; static unsigned int num_ports; -static int irqs[SI_MAX_PARMS]; -static unsigned int num_irqs; -static int regspacings[SI_MAX_PARMS]; -static unsigned int num_regspacings; -static int regsizes[SI_MAX_PARMS]; -static unsigned int num_regsizes; -static int regshifts[SI_MAX_PARMS]; -static unsigned int num_regshifts; -static int slave_addrs[SI_MAX_PARMS]; /* Leaving 0 chooses the default value */ -static unsigned int num_slave_addrs; +static int irqs[SI_MAX_PARMS] __initdata; +static unsigned int num_irqs __initdata; +static int regspacings[SI_MAX_PARMS] __initdata; +static unsigned int num_regspacings __initdata; +static int regsizes[SI_MAX_PARMS]
