Re: [PATCH][next] xfrm: Add error handling when nla_put_u32() returns an error
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Date: 2024-11-13 14:12:13
Also in:
kernel-janitors, lkml
On 11/13/24 00:36, Everest K.C. wrote:
quoted hunk ↗ jump to hunk
Error handling is missing when call to nla_put_u32() fails. Handle the error when the call to nla_put_u32() returns an error. The error was reported by Coverity Scan. Report: CID 1601525: (#1 of 1): Unused value (UNUSED_VALUE) returned_value: Assigning value from nla_put_u32(skb, XFRMA_SA_PCPU, x->pcpu_num) to err here, but that stored value is overwritten before it can be used Fixes: 1ddf9916ac09 ("xfrm: Add support for per cpu xfrm state handling.") Signed-off-by: Everest K.C. <redacted> --- net/xfrm/xfrm_user.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c index f0ee0c7a59dd..a784598cc7cf 100644 --- a/net/xfrm/xfrm_user.c +++ b/net/xfrm/xfrm_user.c@@ -2607,9 +2607,12 @@ static int build_aevent(struct sk_buff *skb, struct xfrm_state *x, const struct err = xfrm_if_id_put(skb, x->if_id); if (err) goto out_cancel; - if (x->pcpu_num != UINT_MAX) + if (x->pcpu_num != UINT_MAX) { err = nla_put_u32(skb, XFRMA_SA_PCPU, x->pcpu_num); - + if (err) + goto out_cancel; + } + if (x->dir) { err = nla_put_u8(skb, XFRMA_SA_DIR, x->dir); if (err)
this is a fix indeed, Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> -- I find nla_put*() familiy error handling very ugly for the calling code, especially given that some of the calls are conditional I would like to refactor it some day, to give the caller possibility to just put all the needed fields and check the error once at the end. Nesting complicates things a bit, but perhaps it could be also covered in such way (didn't checked yet).