linux-kernel - Re: [PATCH] HID: intel_ish-hid: Add check for ishtp_dma_tx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <nycvar.YFH.7.76.2212201550060.9000@cbobk.fhfr.pm>
Date:   Tue, 20 Dec 2022 15:51:17 +0100 (CET)
From:   Jiri Kosina <jikos@...nel.org>
To:     Jiasheng Jiang <jiasheng@...as.ac.cn>
cc:     srinivas.pandruvada@...ux.intel.com, benjamin.tissoires@...hat.com,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] HID: intel_ish-hid: Add check for ishtp_dma_tx_map

On Tue, 22 Nov 2022, Jiasheng Jiang wrote:

> As the kcalloc may return NULL pointer,
> it should be better to check the ishtp_dma_tx_map
> before use in order to avoid NULL pointer dereference.
> 
> Fixes: 3703f53b99e4 ("HID: intel_ish-hid: ISH Transport layer")
> Signed-off-by: Jiasheng Jiang <jiasheng@...as.ac.cn>

Srinivas, can I get your Ack on this one, please?

I'd much prefer to perform the check right at the allocation time, but 
that would need some more code refactoring (as 
there is currently no way for ishtp_cl_alloc_dma_buf() to fail).

> ---
>  drivers/hid/intel-ish-hid/ishtp/dma-if.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/hid/intel-ish-hid/ishtp/dma-if.c b/drivers/hid/intel-ish-hid/ishtp/dma-if.c
> index 40554c8daca0..00046cbfd4ed 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/dma-if.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/dma-if.c
> @@ -104,6 +104,11 @@ void *ishtp_cl_get_dma_send_buf(struct ishtp_device *dev,
>  	int required_slots = (size / DMA_SLOT_SIZE)
>  		+ 1 * (size % DMA_SLOT_SIZE != 0);
>  
> +	if (!dev->ishtp_dma_tx_map) {
> +		dev_err(dev->devc, "Fail to allocate Tx map\n");

I'd also suggest to use "Failed to ..." instead.

Thanks,

-- 
Jiri Kosina
SUSE Labs