Received-SPF: pass (google.com: domain of linux-nfs+bounces-851-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <4307bce9-ccbd-4bc5-aa8e-b618a1664cbe@web.de>
Date: Mon, 1 Jan 2024 12:24:59 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] sunrpc: Improve exception handling in krb5_etm_checksum()
Content-Language: en-GB
To: Chuck Lever <chuck.lever@oracle.com>, linux-nfs@vger.kernel.org,
 netdev@vger.kernel.org, kernel-janitors@vger.kernel.org
Cc: Anna Schumaker <anna@kernel.org>, Ard Biesheuvel <ardb@kernel.org>,
 Dai Ngo <Dai.Ngo@oracle.com>, "David S. Miller" <davem@davemloft.net>,
 Eric Dumazet <edumazet@google.com>, Herbert Xu
 <herbert@gondor.apana.org.au>, Jakub Kicinski <kuba@kernel.org>,
 Jeff Layton <jlayton@kernel.org>, Neil Brown <neilb@suse.de>,
 Olga Kornievskaia <kolga@netapp.com>, Paolo Abeni <pabeni@redhat.com>,
 Simo Sorce <simo@redhat.com>, Tom Talpey <tom@talpey.com>,
 Trond Myklebust <trond.myklebust@hammerspace.com>,
 LKML <linux-kernel@vger.kernel.org>
References: <9561c78e-49a2-430c-a611-52806c0cdf25@web.de>
 <ZZIhEJK68Sapos2t@tissot.1015granger.net>
From: Markus Elfring <Markus.Elfring@web.de>
In-Reply-To: <ZZIhEJK68Sapos2t@tissot.1015granger.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:UdF4WRWURZA=;pdsBrnNEagtfYnRIibWLfuZ6qQz
 OgKCfYYf0dvoBc/SZ9UbJEQC9Zpt/RDML186UyhsETArxUvCLaqqQqcnXn6B1cHz+LDh3JBse
 9Zm4+/ss6A5w5oMg+UP9wbWvy5wO419D4YeeUBKKOKqfuMq97xzMNx4XQHuMTzr2MgLMP1/zx
 CHXuriei2XUOtKnCoI/uMzHIGSqVf8IXgRLiudR4J0x2wawzZYoLg5Ef8IcAx8f8sbM88LXUM
 bI4Q3gGkCzUun4ytu9G4qS74O40/MHMfDcZjMnunOJ88A2uuixxwFbwtpCdNZ3nZb1LTbA+aB
 MpFjTkku53lDMjOn68DT1lkemkFSLE8Z/PtaH4Jd1GEX2IDUBhpgjx/eA1MCey2BBTmo6IopJ
 lBeXxgX88ZnO/ESzQspj20X6qd4/yBGroWfdRB5Z4nh/UD+if7///ijVBK4XVCDiQVJeDYP/p
 KcNhA8OTnGVGhBcsqFQfbViYHZb1Fb6IjtFMrudptaLfpkwtHpqTX7Wb2eS36k97PtxmIb85q
 lnC76zIY/IKfmbAUKGQHXMbE5Vmb6qpsEpmmKIZl6seGrORsk/8XZ1yOO0eyOhypUcMwHEX/o
 utxDtkkgQm04nUBddwCodG42Ey4s4JoP0y/fWtN57x2wadJ5Q2/XFfRv91zmL2uszTVRifJi9
 l7E+Vev+J/HDzCDny9b1znh+rf10C2r8LIDyLAH1okZ9b1dnFCbyOroVB5myZ64lFqTIQ4iF8
 HROSUEp8FNuJkChDTfG7BXNnYCYojOKaFyU8VpXBZTQnvg1qybXgPzfO5J8D7px8/jkvHhBdl
 O+HCwp5ivRsBu65NGv91drq5wdKvgNViLP4CpF5NX+k6B/VqZtKJ+wp/hZXVMxan1ABQK5Tes
 04MjYlWf3bDWlDfynN82OCl6qFElmzBgthArfeV5Eyfz2kZiZ1Op/sh6HvBS34s7cQ0SCJt7e
 LuVrEA==

=E2=80=A6
>> Thus use another label.
=E2=80=A6
>> ---
>>  net/sunrpc/auth_gss/gss_krb5_crypto.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
=E2=80=A6
> As has undoubtedly been pointed out in other forums, calling kfree()
> with a NULL argument is perfectly valid.

The function call =E2=80=9Ckfree(NULL)=E2=80=9D is not really useful for e=
rror/exception handling
while it is tolerated at various source code places.


>                                          Since this small GFP_KERNEL
> allocation almost never fails, it's unlikely this change is going to
> make any difference except for readability.

I became curious if development interests can grow for the usage of
an additional label.
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+go=
to+chain+when+leaving+a+function+on+error+when+using+and+releasing+resourc=
es


> Now if we want to clean up the error flows in here to look more
> idiomatic, how about this:
=E2=80=A6
> +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
=E2=80=A6
> @@ -970,8 +970,9 @@ u32 krb5_etm_checksum(struct crypto_sync_skcipher *c=
ipher,
>
>  out_free_ahash:
>  	ahash_request_free(req);
> -out_free_mem:
> +out_free_iv:
>  	kfree(iv);
> +out_free_cksumdata:
>  	kfree_sensitive(checksumdata);
=E2=80=A6

I find it nice that you show another possible adjustment of corresponding =
identifiers.

Regards,
Markus