| Message ID | 20191215175842.30767-1-pakki001@umn.edu |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | hdlcdrv: replace assertion with recovery code | expand |
> However, by returning the error to the caller in case ops is NULL > can avoid the crash. I suggest to improve this change description by choosing also a more imperative wording. > The patch fixes this issue. Please replace this sentence by the tag “Fixes”. Regards, Markus
On Sun, 15 Dec 2019 11:58:41 -0600 Aditya Pakki <pakki001@umn.edu> wrote: > In hdlcdrv_register, failure to register the driver causes a crash. > However, by returning the error to the caller in case ops is NULL > can avoid the crash. The patch fixes this issue. > > Signed-off-by: Aditya Pakki <pakki001@umn.edu> > --- > drivers/net/hamradio/hdlcdrv.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/hamradio/hdlcdrv.c b/drivers/net/hamradio/hdlcdrv.c > index df495b5595f5..38e5d1e54800 100644 > --- a/drivers/net/hamradio/hdlcdrv.c > +++ b/drivers/net/hamradio/hdlcdrv.c > @@ -687,7 +687,8 @@ struct net_device *hdlcdrv_register(const struct hdlcdrv_ops *ops, > struct hdlcdrv_state *s;h > int err; > > - BUG_ON(ops == NULL); > + if (!ops) > + return ERR_PTR(-EINVAL); > > if (privsize < sizeof(struct hdlcdrv_state)) > privsize = sizeof(struct hdlcdrv_state); It is good to remove BUG_ON's but this is not a good way to fix it. The original code was being over paranoid. There are only 3 places this function is called in the current kernel and all pass a valid pointer. Better just remove the BUG_ON all together; it is not worth carrying bug checks for "some day somebody might add broken code".
diff --git a/drivers/net/hamradio/hdlcdrv.c b/drivers/net/hamradio/hdlcdrv.c index df495b5595f5..38e5d1e54800 100644 --- a/drivers/net/hamradio/hdlcdrv.c +++ b/drivers/net/hamradio/hdlcdrv.c @@ -687,7 +687,8 @@ struct net_device *hdlcdrv_register(const struct hdlcdrv_ops *ops, struct hdlcdrv_state *s; int err; - BUG_ON(ops == NULL); + if (!ops) + return ERR_PTR(-EINVAL); if (privsize < sizeof(struct hdlcdrv_state)) privsize = sizeof(struct hdlcdrv_state);
In hdlcdrv_register, failure to register the driver causes a crash. However, by returning the error to the caller in case ops is NULL can avoid the crash. The patch fixes this issue. Signed-off-by: Aditya Pakki <pakki001@umn.edu> --- drivers/net/hamradio/hdlcdrv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)