Messages in this thread | | | Subject | Re: [PATCH] drivers/char/ipmi_si: prevent null deref during module exit | From | Andrew Banman <> | Date | Wed, 8 Nov 2017 11:11:10 -0600 |
| |
On 11/8/17 11:06 AM, Andrew Banman wrote: > If there are uninitialized SMIs in the smi_infos list, i.e. with no > handlers set, then disable_si_irq() in cleanup_smi_one() will hit a null > pointer dereference when the former attempts to start the check enables > transaction. Thus, we panic during module exit.
I think this points to a broader problem of holding uninitialized smi_info structs in smi_infos list. There are many places where handlers and other struct members are assumed. Maybe a better design would be to remove SMIs from the list if we have no intention of initializing them?
Andrew
> > Avoid panicking when there are uninitialized SMIs by checking for a handler > pointer before starting the check enables transaction. > > Signed-off-by: Andrew Banman <abanman@hpe.com> > --- > drivers/char/ipmi/ipmi_si_intf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c > index cb5719e..6c0b1b3 100644 > --- a/drivers/char/ipmi/ipmi_si_intf.c > +++ b/drivers/char/ipmi/ipmi_si_intf.c > @@ -442,7 +442,7 @@ static void start_check_enables(struct smi_info *smi_info, > bool start_timer) > > if (start_timer) > start_new_msg(smi_info, msg, 2); > - else > + else if (smi_info->handlers) > smi_info->handlers->start_transaction(smi_info->si_sm, msg, 2); > smi_info->si_state = SI_CHECKING_ENABLES; > }
| |