| Message ID | 20230925085318.1228225-1-nichen@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | fjes: Add missing check for vzalloc | expand |
Chen Ni <nichen@iscas.ac.cn> writes: > Because of the potential failure of the vzalloc(), the hw->hw_info.trace > could be NULL. > Therefore, we need to check it and return -ENOMEM in order to transfer > the error. > > Fixes: b6ba737d0b29 ("fjes: ethtool -w and -W support for fjes driver") > Signed-off-by: Chen Ni <nichen@iscas.ac.cn> > --- > drivers/net/fjes/fjes_hw.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/fjes/fjes_hw.c b/drivers/net/fjes/fjes_hw.c > index 704e949484d0..3a06a3cf021d 100644 > --- a/drivers/net/fjes/fjes_hw.c > +++ b/drivers/net/fjes/fjes_hw.c > @@ -330,6 +330,9 @@ int fjes_hw_init(struct fjes_hw *hw) > ret = fjes_hw_setup(hw); > > hw->hw_info.trace = vzalloc(FJES_DEBUG_BUFFER_SIZE); > + if (!hw->hw_info.trace) > + return -ENOMEM; > + I'm not sure, but shouldn't this call fjes_hw_cleanup() to mirror the setup() above? Also only if ret=0 I suppose. > hw->hw_info.trace_size = FJES_DEBUG_BUFFER_SIZE; > > return ret;
On Mon, 2023-09-25 at 16:40 +0200, Petr Machata wrote: > Chen Ni <nichen@iscas.ac.cn> writes: > > > Because of the potential failure of the vzalloc(), the hw->hw_info.trace > > could be NULL. > > Therefore, we need to check it and return -ENOMEM in order to transfer > > the error. > > > > Fixes: b6ba737d0b29 ("fjes: ethtool -w and -W support for fjes driver") > > Signed-off-by: Chen Ni <nichen@iscas.ac.cn> > > --- > > drivers/net/fjes/fjes_hw.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/net/fjes/fjes_hw.c b/drivers/net/fjes/fjes_hw.c > > index 704e949484d0..3a06a3cf021d 100644 > > --- a/drivers/net/fjes/fjes_hw.c > > +++ b/drivers/net/fjes/fjes_hw.c > > @@ -330,6 +330,9 @@ int fjes_hw_init(struct fjes_hw *hw) > > ret = fjes_hw_setup(hw); > > > > hw->hw_info.trace = vzalloc(FJES_DEBUG_BUFFER_SIZE); > > + if (!hw->hw_info.trace) > > + return -ENOMEM; > > + > > I'm not sure, but shouldn't this call fjes_hw_cleanup() to mirror the > setup() above? Also only if ret=0 I suppose. Yes, that looks needed, or memory will be leaked. Additionally it looks like the rest of the driver handles correctly the case where hw_info.trace is NULL, so this fix is likely not needed at all. Cheers, Paolo
diff --git a/drivers/net/fjes/fjes_hw.c b/drivers/net/fjes/fjes_hw.c index 704e949484d0..3a06a3cf021d 100644 --- a/drivers/net/fjes/fjes_hw.c +++ b/drivers/net/fjes/fjes_hw.c @@ -330,6 +330,9 @@ int fjes_hw_init(struct fjes_hw *hw) ret = fjes_hw_setup(hw); hw->hw_info.trace = vzalloc(FJES_DEBUG_BUFFER_SIZE); + if (!hw->hw_info.trace) + return -ENOMEM; + hw->hw_info.trace_size = FJES_DEBUG_BUFFER_SIZE; return ret;
Because of the potential failure of the vzalloc(), the hw->hw_info.trace could be NULL. Therefore, we need to check it and return -ENOMEM in order to transfer the error. Fixes: b6ba737d0b29 ("fjes: ethtool -w and -W support for fjes driver") Signed-off-by: Chen Ni <nichen@iscas.ac.cn> --- drivers/net/fjes/fjes_hw.c | 3 +++ 1 file changed, 3 insertions(+)