On Fri, 26 Nov 2021, Dan Carpenter wrote:
> On Thu, Nov 25, 2021 at 06:18:22PM +0300, Dan Carpenter wrote:
> > I had thought that ->kmap_cnt was a regular int and not an unsigned
> > int, but I would have to pull a stable tree to see where I misread the
> > code.
>
> I was looking at (struct ion_buffer)->kmap_cnt but this is
> (struct ion_handle)->kmap_cnt. I'm not sure how those are related but
> it makes me nervous that one can go higher than the other. Also both
> probably need overflow protection.
>
> So I guess I would just do something like:
>
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 806e9b30b9dc8..e8846279b33b5 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -489,6 +489,8 @@ static void *ion_buffer_kmap_get(struct ion_buffer *buffer)
> void *vaddr;
>
> if (buffer->kmap_cnt) {
> + if (buffer->kmap_cnt == INT_MAX)
> + return ERR_PTR(-EOVERFLOW);
> buffer->kmap_cnt++;
> return buffer->vaddr;
> }
> @@ -509,6 +511,8 @@ static void *ion_handle_kmap_get(struct ion_handle *handle)
> void *vaddr;
>
> if (handle->kmap_cnt) {
> + if (handle->kmap_cnt == INT_MAX)
> + return ERR_PTR(-EOVERFLOW);
> handle->kmap_cnt++;
> return buffer->vaddr;
> }
Which is all well and good until somebody changes the type.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
