* [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int
@ 2021-02-07 23:07 ` Colin King
0 siblings, 0 replies; 20+ messages in thread
From: Colin King @ 2021-02-07 23:07 UTC (permalink / raw)
To: Alex Deucher, Christian König, David Airlie, Daniel Vetter,
Huang Rui, Junwei Zhang, amd-gfx, dri-devel
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The left shift of int 32 bit integer constant 1 is evaluated using 32
bit arithmetic and then assigned to an unsigned 64 bit integer. In the
case where *frag is 32 or more this can lead to an oveflow. Avoid this
by shifting 1ULL.
Addresses-Coverity: ("Unintentional integer overflow")
Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 9d19078246c8..53a925600510 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params,
*frag = max_frag;
*frag_end = end & ~((1ULL << max_frag) - 1);
} else {
- *frag_end = start + (1 << *frag);
+ *frag_end = start + (1ULL << *frag);
}
}
--
2.29.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-07 23:07 ` Colin King 0 siblings, 0 replies; 20+ messages in thread From: Colin King @ 2021-02-07 23:07 UTC (permalink / raw) To: Alex Deucher, Christian König, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx, dri-devel Cc: kernel-janitors, linux-kernel From: Colin Ian King <colin.king@canonical.com> The left shift of int 32 bit integer constant 1 is evaluated using 32 bit arithmetic and then assigned to an unsigned 64 bit integer. In the case where *frag is 32 or more this can lead to an oveflow. Avoid this by shifting 1ULL. Addresses-Coverity: ("Unintentional integer overflow") Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") Signed-off-by: Colin Ian King <colin.king@canonical.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 9d19078246c8..53a925600510 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, *frag = max_frag; *frag_end = end & ~((1ULL << max_frag) - 1); } else { - *frag_end = start + (1 << *frag); + *frag_end = start + (1ULL << *frag); } } -- 2.29.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-07 23:07 ` Colin King 0 siblings, 0 replies; 20+ messages in thread From: Colin King @ 2021-02-07 23:07 UTC (permalink / raw) To: Alex Deucher, Christian König, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx, dri-devel Cc: kernel-janitors, linux-kernel From: Colin Ian King <colin.king@canonical.com> The left shift of int 32 bit integer constant 1 is evaluated using 32 bit arithmetic and then assigned to an unsigned 64 bit integer. In the case where *frag is 32 or more this can lead to an oveflow. Avoid this by shifting 1ULL. Addresses-Coverity: ("Unintentional integer overflow") Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") Signed-off-by: Colin Ian King <colin.king@canonical.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 9d19078246c8..53a925600510 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, *frag = max_frag; *frag_end = end & ~((1ULL << max_frag) - 1); } else { - *frag_end = start + (1 << *frag); + *frag_end = start + (1ULL << *frag); } } -- 2.29.2 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-07 23:07 ` Colin King 0 siblings, 0 replies; 20+ messages in thread From: Colin King @ 2021-02-07 23:07 UTC (permalink / raw) To: Alex Deucher, Christian König, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx, dri-devel Cc: kernel-janitors, linux-kernel From: Colin Ian King <colin.king@canonical.com> The left shift of int 32 bit integer constant 1 is evaluated using 32 bit arithmetic and then assigned to an unsigned 64 bit integer. In the case where *frag is 32 or more this can lead to an oveflow. Avoid this by shifting 1ULL. Addresses-Coverity: ("Unintentional integer overflow") Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") Signed-off-by: Colin Ian King <colin.king@canonical.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 9d19078246c8..53a925600510 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, *frag = max_frag; *frag_end = end & ~((1ULL << max_frag) - 1); } else { - *frag_end = start + (1 << *frag); + *frag_end = start + (1ULL << *frag); } } -- 2.29.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int 2021-02-07 23:07 ` Colin King (?) (?) @ 2021-02-08 9:17 ` Christian König -1 siblings, 0 replies; 20+ messages in thread From: Christian König @ 2021-02-08 9:17 UTC (permalink / raw) To: Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx, dri-devel Cc: kernel-janitors, linux-kernel Am 08.02.21 um 00:07 schrieb Colin King: > From: Colin Ian King <colin.king@canonical.com> > > The left shift of int 32 bit integer constant 1 is evaluated using 32 > bit arithmetic and then assigned to an unsigned 64 bit integer. In the > case where *frag is 32 or more this can lead to an oveflow. Avoid this > by shifting 1ULL. Well that can't happen. Take a look at the code in that function: > max_frag = 31; ... > if (*frag >= max_frag) { > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > *frag_end = start + (1 << *frag); > } But I'm fine with applying the patch if it silences your warning. Regards, Christian. > > Addresses-Coverity: ("Unintentional integer overflow") > Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 9d19078246c8..53a925600510 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > - *frag_end = start + (1 << *frag); > + *frag_end = start + (1ULL << *frag); > } > } > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 9:17 ` Christian König 0 siblings, 0 replies; 20+ messages in thread From: Christian König @ 2021-02-08 9:17 UTC (permalink / raw) To: Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx, dri-devel Cc: kernel-janitors, linux-kernel Am 08.02.21 um 00:07 schrieb Colin King: > From: Colin Ian King <colin.king@canonical.com> > > The left shift of int 32 bit integer constant 1 is evaluated using 32 > bit arithmetic and then assigned to an unsigned 64 bit integer. In the > case where *frag is 32 or more this can lead to an oveflow. Avoid this > by shifting 1ULL. Well that can't happen. Take a look at the code in that function: > max_frag = 31; ... > if (*frag >= max_frag) { > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > *frag_end = start + (1 << *frag); > } But I'm fine with applying the patch if it silences your warning. Regards, Christian. > > Addresses-Coverity: ("Unintentional integer overflow") > Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 9d19078246c8..53a925600510 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > - *frag_end = start + (1 << *frag); > + *frag_end = start + (1ULL << *frag); > } > } > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 9:17 ` Christian König 0 siblings, 0 replies; 20+ messages in thread From: Christian König @ 2021-02-08 9:17 UTC (permalink / raw) To: Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx, dri-devel Cc: kernel-janitors, linux-kernel Am 08.02.21 um 00:07 schrieb Colin King: > From: Colin Ian King <colin.king@canonical.com> > > The left shift of int 32 bit integer constant 1 is evaluated using 32 > bit arithmetic and then assigned to an unsigned 64 bit integer. In the > case where *frag is 32 or more this can lead to an oveflow. Avoid this > by shifting 1ULL. Well that can't happen. Take a look at the code in that function: > max_frag = 31; ... > if (*frag >= max_frag) { > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > *frag_end = start + (1 << *frag); > } But I'm fine with applying the patch if it silences your warning. Regards, Christian. > > Addresses-Coverity: ("Unintentional integer overflow") > Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 9d19078246c8..53a925600510 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > - *frag_end = start + (1 << *frag); > + *frag_end = start + (1ULL << *frag); > } > } > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 9:17 ` Christian König 0 siblings, 0 replies; 20+ messages in thread From: Christian König @ 2021-02-08 9:17 UTC (permalink / raw) To: Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx, dri-devel Cc: kernel-janitors, linux-kernel Am 08.02.21 um 00:07 schrieb Colin King: > From: Colin Ian King <colin.king@canonical.com> > > The left shift of int 32 bit integer constant 1 is evaluated using 32 > bit arithmetic and then assigned to an unsigned 64 bit integer. In the > case where *frag is 32 or more this can lead to an oveflow. Avoid this > by shifting 1ULL. Well that can't happen. Take a look at the code in that function: > max_frag = 31; ... > if (*frag >= max_frag) { > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > *frag_end = start + (1 << *frag); > } But I'm fine with applying the patch if it silences your warning. Regards, Christian. > > Addresses-Coverity: ("Unintentional integer overflow") > Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 9d19078246c8..53a925600510 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > - *frag_end = start + (1 << *frag); > + *frag_end = start + (1ULL << *frag); > } > } > ^ permalink raw reply [flat|nested] 20+ messages in thread
* AW: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int 2021-02-08 9:17 ` Christian König (?) (?) @ 2021-02-08 11:05 ` Walter Harms -1 siblings, 0 replies; 20+ messages in thread From: Walter Harms @ 2021-02-08 11:05 UTC (permalink / raw) To: Christian König, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org i am curious: what is the win to have a unsigned 64 bit integer in the first place ? re, wh ________________________________________ Von: Christian König <christian.koenig@amd.com> Gesendet: Montag, 8. Februar 2021 10:17:42 An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int Am 08.02.21 um 00:07 schrieb Colin King: > From: Colin Ian King <colin.king@canonical.com> > > The left shift of int 32 bit integer constant 1 is evaluated using 32 > bit arithmetic and then assigned to an unsigned 64 bit integer. In the > case where *frag is 32 or more this can lead to an oveflow. Avoid this > by shifting 1ULL. Well that can't happen. Take a look at the code in that function: > max_frag = 31; ... > if (*frag >= max_frag) { > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > *frag_end = start + (1 << *frag); > } But I'm fine with applying the patch if it silences your warning. Regards, Christian. > > Addresses-Coverity: ("Unintentional integer overflow") > Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 9d19078246c8..53a925600510 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > - *frag_end = start + (1 << *frag); > + *frag_end = start + (1ULL << *frag); > } > } > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* AW: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 11:05 ` Walter Harms 0 siblings, 0 replies; 20+ messages in thread From: Walter Harms @ 2021-02-08 11:05 UTC (permalink / raw) To: Christian König, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org i am curious: what is the win to have a unsigned 64 bit integer in the first place ? re, wh ________________________________________ Von: Christian König <christian.koenig@amd.com> Gesendet: Montag, 8. Februar 2021 10:17:42 An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int Am 08.02.21 um 00:07 schrieb Colin King: > From: Colin Ian King <colin.king@canonical.com> > > The left shift of int 32 bit integer constant 1 is evaluated using 32 > bit arithmetic and then assigned to an unsigned 64 bit integer. In the > case where *frag is 32 or more this can lead to an oveflow. Avoid this > by shifting 1ULL. Well that can't happen. Take a look at the code in that function: > max_frag = 31; ... > if (*frag >= max_frag) { > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > *frag_end = start + (1 << *frag); > } But I'm fine with applying the patch if it silences your warning. Regards, Christian. > > Addresses-Coverity: ("Unintentional integer overflow") > Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 9d19078246c8..53a925600510 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > - *frag_end = start + (1 << *frag); > + *frag_end = start + (1ULL << *frag); > } > } > ^ permalink raw reply [flat|nested] 20+ messages in thread
* AW: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 11:05 ` Walter Harms 0 siblings, 0 replies; 20+ messages in thread From: Walter Harms @ 2021-02-08 11:05 UTC (permalink / raw) To: Christian König, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org i am curious: what is the win to have a unsigned 64 bit integer in the first place ? re, wh ________________________________________ Von: Christian König <christian.koenig@amd.com> Gesendet: Montag, 8. Februar 2021 10:17:42 An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int Am 08.02.21 um 00:07 schrieb Colin King: > From: Colin Ian King <colin.king@canonical.com> > > The left shift of int 32 bit integer constant 1 is evaluated using 32 > bit arithmetic and then assigned to an unsigned 64 bit integer. In the > case where *frag is 32 or more this can lead to an oveflow. Avoid this > by shifting 1ULL. Well that can't happen. Take a look at the code in that function: > max_frag = 31; ... > if (*frag >= max_frag) { > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > *frag_end = start + (1 << *frag); > } But I'm fine with applying the patch if it silences your warning. Regards, Christian. > > Addresses-Coverity: ("Unintentional integer overflow") > Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 9d19078246c8..53a925600510 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > - *frag_end = start + (1 << *frag); > + *frag_end = start + (1ULL << *frag); > } > } > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* AW: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 11:05 ` Walter Harms 0 siblings, 0 replies; 20+ messages in thread From: Walter Harms @ 2021-02-08 11:05 UTC (permalink / raw) To: Christian König, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org i am curious: what is the win to have a unsigned 64 bit integer in the first place ? re, wh ________________________________________ Von: Christian König <christian.koenig@amd.com> Gesendet: Montag, 8. Februar 2021 10:17:42 An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int Am 08.02.21 um 00:07 schrieb Colin King: > From: Colin Ian King <colin.king@canonical.com> > > The left shift of int 32 bit integer constant 1 is evaluated using 32 > bit arithmetic and then assigned to an unsigned 64 bit integer. In the > case where *frag is 32 or more this can lead to an oveflow. Avoid this > by shifting 1ULL. Well that can't happen. Take a look at the code in that function: > max_frag = 31; ... > if (*frag >= max_frag) { > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > *frag_end = start + (1 << *frag); > } But I'm fine with applying the patch if it silences your warning. Regards, Christian. > > Addresses-Coverity: ("Unintentional integer overflow") > Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 9d19078246c8..53a925600510 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, > *frag = max_frag; > *frag_end = end & ~((1ULL << max_frag) - 1); > } else { > - *frag_end = start + (1 << *frag); > + *frag_end = start + (1ULL << *frag); > } > } > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int 2021-02-08 11:05 ` Walter Harms (?) (?) @ 2021-02-08 12:14 ` Christian König -1 siblings, 0 replies; 20+ messages in thread From: Christian König @ 2021-02-08 12:14 UTC (permalink / raw) To: Walter Harms, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org For start and end? The hardware has 48 bit address space and that won't fit into 32bits. Only the fragment handling can't do more than 2GB at the same time. Christian. Am 08.02.21 um 12:05 schrieb Walter Harms: > i am curious: > what is the win to have a unsigned 64 bit integer in the first > place ? > > re, > wh > ________________________________________ > Von: Christian König <christian.koenig@amd.com> > Gesendet: Montag, 8. Februar 2021 10:17:42 > An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org > Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int > > Am 08.02.21 um 00:07 schrieb Colin King: >> From: Colin Ian King <colin.king@canonical.com> >> >> The left shift of int 32 bit integer constant 1 is evaluated using 32 >> bit arithmetic and then assigned to an unsigned 64 bit integer. In the >> case where *frag is 32 or more this can lead to an oveflow. Avoid this >> by shifting 1ULL. > Well that can't happen. Take a look at the code in that function: > >> max_frag = 31; > ... >> if (*frag >= max_frag) { >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> *frag_end = start + (1 << *frag); >> } > But I'm fine with applying the patch if it silences your warning. > > Regards, > Christian. > >> Addresses-Coverity: ("Unintentional integer overflow") >> Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 9d19078246c8..53a925600510 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> - *frag_end = start + (1 << *frag); >> + *frag_end = start + (1ULL << *frag); >> } >> } >> _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 12:14 ` Christian König 0 siblings, 0 replies; 20+ messages in thread From: Christian König @ 2021-02-08 12:14 UTC (permalink / raw) To: Walter Harms, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org For start and end? The hardware has 48 bit address space and that won't fit into 32bits. Only the fragment handling can't do more than 2GB at the same time. Christian. Am 08.02.21 um 12:05 schrieb Walter Harms: > i am curious: > what is the win to have a unsigned 64 bit integer in the first > place ? > > re, > wh > ________________________________________ > Von: Christian König <christian.koenig@amd.com> > Gesendet: Montag, 8. Februar 2021 10:17:42 > An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org > Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int > > Am 08.02.21 um 00:07 schrieb Colin King: >> From: Colin Ian King <colin.king@canonical.com> >> >> The left shift of int 32 bit integer constant 1 is evaluated using 32 >> bit arithmetic and then assigned to an unsigned 64 bit integer. In the >> case where *frag is 32 or more this can lead to an oveflow. Avoid this >> by shifting 1ULL. > Well that can't happen. Take a look at the code in that function: > >> max_frag = 31; > ... >> if (*frag >= max_frag) { >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> *frag_end = start + (1 << *frag); >> } > But I'm fine with applying the patch if it silences your warning. > > Regards, > Christian. > >> Addresses-Coverity: ("Unintentional integer overflow") >> Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 9d19078246c8..53a925600510 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> - *frag_end = start + (1 << *frag); >> + *frag_end = start + (1ULL << *frag); >> } >> } >> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 12:14 ` Christian König 0 siblings, 0 replies; 20+ messages in thread From: Christian König @ 2021-02-08 12:14 UTC (permalink / raw) To: Walter Harms, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org For start and end? The hardware has 48 bit address space and that won't fit into 32bits. Only the fragment handling can't do more than 2GB at the same time. Christian. Am 08.02.21 um 12:05 schrieb Walter Harms: > i am curious: > what is the win to have a unsigned 64 bit integer in the first > place ? > > re, > wh > ________________________________________ > Von: Christian König <christian.koenig@amd.com> > Gesendet: Montag, 8. Februar 2021 10:17:42 > An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org > Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int > > Am 08.02.21 um 00:07 schrieb Colin King: >> From: Colin Ian King <colin.king@canonical.com> >> >> The left shift of int 32 bit integer constant 1 is evaluated using 32 >> bit arithmetic and then assigned to an unsigned 64 bit integer. In the >> case where *frag is 32 or more this can lead to an oveflow. Avoid this >> by shifting 1ULL. > Well that can't happen. Take a look at the code in that function: > >> max_frag = 31; > ... >> if (*frag >= max_frag) { >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> *frag_end = start + (1 << *frag); >> } > But I'm fine with applying the patch if it silences your warning. > > Regards, > Christian. > >> Addresses-Coverity: ("Unintentional integer overflow") >> Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 9d19078246c8..53a925600510 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> - *frag_end = start + (1 << *frag); >> + *frag_end = start + (1ULL << *frag); >> } >> } >> _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 12:14 ` Christian König 0 siblings, 0 replies; 20+ messages in thread From: Christian König @ 2021-02-08 12:14 UTC (permalink / raw) To: Walter Harms, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org For start and end? The hardware has 48 bit address space and that won't fit into 32bits. Only the fragment handling can't do more than 2GB at the same time. Christian. Am 08.02.21 um 12:05 schrieb Walter Harms: > i am curious: > what is the win to have a unsigned 64 bit integer in the first > place ? > > re, > wh > ________________________________________ > Von: Christian König <christian.koenig@amd.com> > Gesendet: Montag, 8. Februar 2021 10:17:42 > An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org > Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int > > Am 08.02.21 um 00:07 schrieb Colin King: >> From: Colin Ian King <colin.king@canonical.com> >> >> The left shift of int 32 bit integer constant 1 is evaluated using 32 >> bit arithmetic and then assigned to an unsigned 64 bit integer. In the >> case where *frag is 32 or more this can lead to an oveflow. Avoid this >> by shifting 1ULL. > Well that can't happen. Take a look at the code in that function: > >> max_frag = 31; > ... >> if (*frag >= max_frag) { >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> *frag_end = start + (1 << *frag); >> } > But I'm fine with applying the patch if it silences your warning. > > Regards, > Christian. > >> Addresses-Coverity: ("Unintentional integer overflow") >> Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 9d19078246c8..53a925600510 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> - *frag_end = start + (1 << *frag); >> + *frag_end = start + (1ULL << *frag); >> } >> } >> ^ permalink raw reply [flat|nested] 20+ messages in thread
* AW: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int 2021-02-08 12:14 ` Christian König (?) (?) @ 2021-02-08 15:55 ` Walter Harms -1 siblings, 0 replies; 20+ messages in thread From: Walter Harms @ 2021-02-08 15:55 UTC (permalink / raw) To: Christian König, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org thx for info ________________________________________ Von: Christian König <christian.koenig@amd.com> Gesendet: Montag, 8. Februar 2021 13:14:49 An: Walter Harms; Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int For start and end? The hardware has 48 bit address space and that won't fit into 32bits. Only the fragment handling can't do more than 2GB at the same time. Christian. Am 08.02.21 um 12:05 schrieb Walter Harms: > i am curious: > what is the win to have a unsigned 64 bit integer in the first > place ? > > re, > wh > ________________________________________ > Von: Christian König <christian.koenig@amd.com> > Gesendet: Montag, 8. Februar 2021 10:17:42 > An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org > Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int > > Am 08.02.21 um 00:07 schrieb Colin King: >> From: Colin Ian King <colin.king@canonical.com> >> >> The left shift of int 32 bit integer constant 1 is evaluated using 32 >> bit arithmetic and then assigned to an unsigned 64 bit integer. In the >> case where *frag is 32 or more this can lead to an oveflow. Avoid this >> by shifting 1ULL. > Well that can't happen. Take a look at the code in that function: > >> max_frag = 31; > ... >> if (*frag >= max_frag) { >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> *frag_end = start + (1 << *frag); >> } > But I'm fine with applying the patch if it silences your warning. > > Regards, > Christian. > >> Addresses-Coverity: ("Unintentional integer overflow") >> Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 9d19078246c8..53a925600510 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> - *frag_end = start + (1 << *frag); >> + *frag_end = start + (1ULL << *frag); >> } >> } >> _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* AW: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 15:55 ` Walter Harms 0 siblings, 0 replies; 20+ messages in thread From: Walter Harms @ 2021-02-08 15:55 UTC (permalink / raw) To: Christian König, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org thx for info ________________________________________ Von: Christian König <christian.koenig@amd.com> Gesendet: Montag, 8. Februar 2021 13:14:49 An: Walter Harms; Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int For start and end? The hardware has 48 bit address space and that won't fit into 32bits. Only the fragment handling can't do more than 2GB at the same time. Christian. Am 08.02.21 um 12:05 schrieb Walter Harms: > i am curious: > what is the win to have a unsigned 64 bit integer in the first > place ? > > re, > wh > ________________________________________ > Von: Christian König <christian.koenig@amd.com> > Gesendet: Montag, 8. Februar 2021 10:17:42 > An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org > Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int > > Am 08.02.21 um 00:07 schrieb Colin King: >> From: Colin Ian King <colin.king@canonical.com> >> >> The left shift of int 32 bit integer constant 1 is evaluated using 32 >> bit arithmetic and then assigned to an unsigned 64 bit integer. In the >> case where *frag is 32 or more this can lead to an oveflow. Avoid this >> by shifting 1ULL. > Well that can't happen. Take a look at the code in that function: > >> max_frag = 31; > ... >> if (*frag >= max_frag) { >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> *frag_end = start + (1 << *frag); >> } > But I'm fine with applying the patch if it silences your warning. > > Regards, > Christian. > >> Addresses-Coverity: ("Unintentional integer overflow") >> Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 9d19078246c8..53a925600510 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> - *frag_end = start + (1 << *frag); >> + *frag_end = start + (1ULL << *frag); >> } >> } >> ^ permalink raw reply [flat|nested] 20+ messages in thread
* AW: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 15:55 ` Walter Harms 0 siblings, 0 replies; 20+ messages in thread From: Walter Harms @ 2021-02-08 15:55 UTC (permalink / raw) To: Christian König, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org thx for info ________________________________________ Von: Christian König <christian.koenig@amd.com> Gesendet: Montag, 8. Februar 2021 13:14:49 An: Walter Harms; Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int For start and end? The hardware has 48 bit address space and that won't fit into 32bits. Only the fragment handling can't do more than 2GB at the same time. Christian. Am 08.02.21 um 12:05 schrieb Walter Harms: > i am curious: > what is the win to have a unsigned 64 bit integer in the first > place ? > > re, > wh > ________________________________________ > Von: Christian König <christian.koenig@amd.com> > Gesendet: Montag, 8. Februar 2021 10:17:42 > An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org > Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int > > Am 08.02.21 um 00:07 schrieb Colin King: >> From: Colin Ian King <colin.king@canonical.com> >> >> The left shift of int 32 bit integer constant 1 is evaluated using 32 >> bit arithmetic and then assigned to an unsigned 64 bit integer. In the >> case where *frag is 32 or more this can lead to an oveflow. Avoid this >> by shifting 1ULL. > Well that can't happen. Take a look at the code in that function: > >> max_frag = 31; > ... >> if (*frag >= max_frag) { >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> *frag_end = start + (1 << *frag); >> } > But I'm fine with applying the patch if it silences your warning. > > Regards, > Christian. > >> Addresses-Coverity: ("Unintentional integer overflow") >> Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 9d19078246c8..53a925600510 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> - *frag_end = start + (1 << *frag); >> + *frag_end = start + (1ULL << *frag); >> } >> } >> _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* AW: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int @ 2021-02-08 15:55 ` Walter Harms 0 siblings, 0 replies; 20+ messages in thread From: Walter Harms @ 2021-02-08 15:55 UTC (permalink / raw) To: Christian König, Colin King, Alex Deucher, David Airlie, Daniel Vetter, Huang Rui, Junwei Zhang, amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org thx for info ________________________________________ Von: Christian König <christian.koenig@amd.com> Gesendet: Montag, 8. Februar 2021 13:14:49 An: Walter Harms; Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int For start and end? The hardware has 48 bit address space and that won't fit into 32bits. Only the fragment handling can't do more than 2GB at the same time. Christian. Am 08.02.21 um 12:05 schrieb Walter Harms: > i am curious: > what is the win to have a unsigned 64 bit integer in the first > place ? > > re, > wh > ________________________________________ > Von: Christian König <christian.koenig@amd.com> > Gesendet: Montag, 8. Februar 2021 10:17:42 > An: Colin King; Alex Deucher; David Airlie; Daniel Vetter; Huang Rui; Junwei Zhang; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org > Betreff: Re: [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int > > Am 08.02.21 um 00:07 schrieb Colin King: >> From: Colin Ian King <colin.king@canonical.com> >> >> The left shift of int 32 bit integer constant 1 is evaluated using 32 >> bit arithmetic and then assigned to an unsigned 64 bit integer. In the >> case where *frag is 32 or more this can lead to an oveflow. Avoid this >> by shifting 1ULL. > Well that can't happen. Take a look at the code in that function: > >> max_frag = 31; > ... >> if (*frag >= max_frag) { >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> *frag_end = start + (1 << *frag); >> } > But I'm fine with applying the patch if it silences your warning. > > Regards, > Christian. > >> Addresses-Coverity: ("Unintentional integer overflow") >> Fixes: dfcd99f6273e ("drm/amdgpu: meld together VM fragment and huge page handling") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 9d19078246c8..53a925600510 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1412,7 +1412,7 @@ static void amdgpu_vm_fragment(struct amdgpu_vm_update_params *params, >> *frag = max_frag; >> *frag_end = end & ~((1ULL << max_frag) - 1); >> } else { >> - *frag_end = start + (1 << *frag); >> + *frag_end = start + (1ULL << *frag); >> } >> } >> ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-02-08 18:29 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-02-07 23:07 [PATCH] drm/amdgpu: fix potential integer overflow on shift of a int Colin King 2021-02-07 23:07 ` Colin King 2021-02-07 23:07 ` Colin King 2021-02-07 23:07 ` Colin King 2021-02-08 9:17 ` Christian König 2021-02-08 9:17 ` Christian König 2021-02-08 9:17 ` Christian König 2021-02-08 9:17 ` Christian König 2021-02-08 11:05 ` AW: " Walter Harms 2021-02-08 11:05 ` Walter Harms 2021-02-08 11:05 ` Walter Harms 2021-02-08 11:05 ` Walter Harms 2021-02-08 12:14 ` Christian König 2021-02-08 12:14 ` Christian König 2021-02-08 12:14 ` Christian König 2021-02-08 12:14 ` Christian König 2021-02-08 15:55 ` AW: " Walter Harms 2021-02-08 15:55 ` Walter Harms 2021-02-08 15:55 ` Walter Harms 2021-02-08 15:55 ` Walter Harms
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.