| Message ID | 20250321164126.329638-1-xiaogang.chen@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | udmabuf: fix a buf size overflow issue during udmabuf creation | expand |
Am 21.03.25 um 17:41 schrieb Xiaogang.Chen: > From: Xiaogang Chen <xiaogang.chen@amd.com> > > by casting size_limit_mb to u64 when calculate pglimit. > > Signed-off-by: Xiaogang Chen<Xiaogang.Chen@amd.com> Reviewed-by: Christian König <christian.koenig@amd.com> If nobody objects I'm going to push that to drm-misc-fixes. Regards, Christian. > --- > drivers/dma-buf/udmabuf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index 8ce1f074c2d3..e99e3a65a470 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -398,7 +398,7 @@ static long udmabuf_create(struct miscdevice *device, > if (!ubuf) > return -ENOMEM; > > - pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; > + pglimit = ((u64)size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; > for (i = 0; i < head->count; i++) { > pgoff_t subpgcnt; >
Hi Christian, > Am 21.03.25 um 17:41 schrieb Xiaogang.Chen: > > From: Xiaogang Chen <xiaogang.chen@amd.com> > > > > by casting size_limit_mb to u64 when calculate pglimit. > > > > Signed-off-by: Xiaogang Chen<Xiaogang.Chen@amd.com> > > Reviewed-by: Christian König <christian.koenig@amd.com> > > If nobody objects I'm going to push that to drm-misc-fixes. No objection but I wish the author would have added more details in the commit message particularly the value they have used to trigger the overflow. I guess Xiaogang can still comment here and briefly describe the exact use-case/test-case they are running where they encountered this issue. Thanks, Vivek > > Regards, > Christian. > > > --- > > drivers/dma-buf/udmabuf.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > > index 8ce1f074c2d3..e99e3a65a470 100644 > > --- a/drivers/dma-buf/udmabuf.c > > +++ b/drivers/dma-buf/udmabuf.c > > @@ -398,7 +398,7 @@ static long udmabuf_create(struct miscdevice > *device, > > if (!ubuf) > > return -ENOMEM; > > > > - pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; > > + pglimit = ((u64)size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; > > for (i = 0; i < head->count; i++) { > > pgoff_t subpgcnt; > >
Am 25.03.25 um 07:23 schrieb Kasireddy, Vivek: > Hi Christian, > >> Am 21.03.25 um 17:41 schrieb Xiaogang.Chen: >>> From: Xiaogang Chen <xiaogang.chen@amd.com> >>> >>> by casting size_limit_mb to u64 when calculate pglimit. >>> >>> Signed-off-by: Xiaogang Chen<Xiaogang.Chen@amd.com> >> Reviewed-by: Christian König <christian.koenig@amd.com> >> >> If nobody objects I'm going to push that to drm-misc-fixes. > No objection but I wish the author would have added more details in the commit > message particularly the value they have used to trigger the overflow. I guess > Xiaogang can still comment here and briefly describe the exact use-case/test-case > they are running where they encountered this issue. Isn't that obvious? At least it was for me. As soon as you have a value larger than 4095 the 32bit multiplication overflows, resulting in incorrectly limiting the buffer size. Regards, Christian. > > Thanks, > Vivek > >> Regards, >> Christian. >> >>> --- >>> drivers/dma-buf/udmabuf.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c >>> index 8ce1f074c2d3..e99e3a65a470 100644 >>> --- a/drivers/dma-buf/udmabuf.c >>> +++ b/drivers/dma-buf/udmabuf.c >>> @@ -398,7 +398,7 @@ static long udmabuf_create(struct miscdevice >> *device, >>> if (!ubuf) >>> return -ENOMEM; >>> >>> - pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; >>> + pglimit = ((u64)size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; >>> for (i = 0; i < head->count; i++) { >>> pgoff_t subpgcnt; >>>
Hi Christian, > Subject: Re: [PATCH] udmabuf: fix a buf size overflow issue during udmabuf > creation > > Am 25.03.25 um 07:23 schrieb Kasireddy, Vivek: > > Hi Christian, > > > >> Am 21.03.25 um 17:41 schrieb Xiaogang.Chen: > >>> From: Xiaogang Chen <xiaogang.chen@amd.com> > >>> > >>> by casting size_limit_mb to u64 when calculate pglimit. > >>> > >>> Signed-off-by: Xiaogang Chen<Xiaogang.Chen@amd.com> > >> Reviewed-by: Christian König <christian.koenig@amd.com> > >> > >> If nobody objects I'm going to push that to drm-misc-fixes. > > No objection but I wish the author would have added more details in the > commit > > message particularly the value they have used to trigger the overflow. I > guess > > Xiaogang can still comment here and briefly describe the exact use- > case/test-case > > they are running where they encountered this issue. > > Isn't that obvious? At least it was for me. > > As soon as you have a value larger than 4095 the 32bit multiplication > overflows, resulting in incorrectly limiting the buffer size. Right, that part makes sense. I was mostly curious about why or how they were using such a large buffer (use-case details). Thanks, Vivek > > Regards, > Christian. > > > > > Thanks, > > Vivek > > > >> Regards, > >> Christian. > >> > >>> --- > >>> drivers/dma-buf/udmabuf.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > >>> index 8ce1f074c2d3..e99e3a65a470 100644 > >>> --- a/drivers/dma-buf/udmabuf.c > >>> +++ b/drivers/dma-buf/udmabuf.c > >>> @@ -398,7 +398,7 @@ static long udmabuf_create(struct miscdevice > >> *device, > >>> if (!ubuf) > >>> return -ENOMEM; > >>> > >>> - pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; > >>> + pglimit = ((u64)size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; > >>> for (i = 0; i < head->count; i++) { > >>> pgoff_t subpgcnt; > >>>
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 8ce1f074c2d3..e99e3a65a470 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -398,7 +398,7 @@ static long udmabuf_create(struct miscdevice *device, if (!ubuf) return -ENOMEM; - pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; + pglimit = ((u64)size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; for (i = 0; i < head->count; i++) { pgoff_t subpgcnt;