Re: [PATCH] HID: intel_ish-hid: Add check for ishtp_dma_tx_map

From: Jiri Kosina
Date: Tue Dec 20 2022 - 09:52:13 EST


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@xxxxxxxxxxx>

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