On Fri, Nov 26, 2021 at 08:56:27AM +0000, Lee Jones wrote:
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);@@ -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);Which is all well and good until somebody changes the type.
It would only break if they change it to something smaller than an int. I have zero sympathy for them if they do that.
regards, dan carpenter