Am Montag, 16. September 2024, 03:38:56 GMT-5 schrieb Herbert Xu:
Hi George,
On Mon, Sep 16, 2024 at 10:44:22AM +0300, George Rurikov wrote:
From: MrRurikov grurikovsherbakov@yandex.ru
After having been compared to a NULL value at algif_aead.c:191, pointer 'tsgl_src' is passed as 2nd parameter in call to function 'crypto_aead_copy_sgl' at algif_aead.c:244, where it is dereferenced at algif_aead.c:85.
Change logical operator from && to || because pointer 'tsgl_src' is NULL, then 'proccessed' will still be non-null
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Cc: stable@vger.kernel.org Fixes: 2d97591ef43d ("crypto: af_alg - consolidation of duplicate code") Signed-off-by: MrRurikov grurikovsherbakov@yandex.ru
crypto/algif_aead.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Cc Stephan.
I am not sure that this is a valid finding. An issue exists when there is processed != 0 and TSGL is NULL. Otherwise, the subsequent copy operation for this part will simply copy nothing: even if TSGL is NULL, the processed value is 0 and this is uses as the length parameter in the copy operation. Technically any copy operation is prevented in the following code that is invoked by the used crypto_null cipher:
static int skcipher_walk_skcipher(struct skcipher_walk *walk, struct skcipher_request *req) { ... /* here we have the value of processed */ walk->total = req->cryptlen;
... /* here we stop processing */ if (unlikely(!walk->total)) return 0;
/* here we dereference the TSGL */ scatterwalk_start(&walk->in, req->src);
You see, the processing stops before the dereferencing.
In any case, the check as it currently is, allows the use of, say, you request a tag from just the key without any AAD or input data. Mathematically this is a valid operation.
Thus, as of now I do not see (a) a technical issue and (b) a mathematical issue.
Could you please help me understand the issue you think you are seeing?
Ciao Stephan