| Message ID | 20230821061944.197934-1-suhui@nfschina.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amdgpu: Avoid possible buffer overflow | expand |
Le 21/08/2023 à 08:19, Su Hui a écrit : > smatch error: > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1257 amdgpu_discovery_reg_base_init() error: > testing array offset 'adev->vcn.num_vcn_inst' after use. > > change the assignment order to avoid buffer overflow. > > Fixes: c40bdfb2ffa4 ("drm/amdgpu: fix incorrect VCN revision in SRIOV") > Signed-off-by: Su Hui <suhui@nfschina.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > index 8e1cfc87122d..ba95526c3d45 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > @@ -1250,11 +1250,12 @@ static int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev) > * 0b10 : encode is disabled > * 0b01 : decode is disabled > */ > - adev->vcn.vcn_config[adev->vcn.num_vcn_inst] = > - ip->revision & 0xc0; > + > ip->revision &= ~0xc0; > if (adev->vcn.num_vcn_inst < > AMDGPU_MAX_VCN_INSTANCES) { > + adev->vcn.vcn_config[adev->vcn.num_vcn_inst] = > + ip->revision & 0xc0; Hi, I don't think that the patch is correct. Because of the "ip->revision &= ~0xc0" which is now above it, "ip->revision & 0xc0" should now always be 0. Maybe both lines should be moved within the "if"? CJ > adev->vcn.num_vcn_inst++; > adev->vcn.inst_mask |= > (1U << ip->instance_number);
On 2023/8/21 14:47, Christophe JAILLET wrote: > Le 21/08/2023 à 08:19, Su Hui a écrit : >> smatch error: >> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1257 >> amdgpu_discovery_reg_base_init() error: >> testing array offset 'adev->vcn.num_vcn_inst' after use. >> >> change the assignment order to avoid buffer overflow. >> >> Fixes: c40bdfb2ffa4 ("drm/amdgpu: fix incorrect VCN revision in SRIOV") >> Signed-off-by: Su Hui <suhui@nfschina.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c >> index 8e1cfc87122d..ba95526c3d45 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c >> @@ -1250,11 +1250,12 @@ static int >> amdgpu_discovery_reg_base_init(struct amdgpu_device *adev) >> * 0b10 : encode is disabled >> * 0b01 : decode is disabled >> */ >> - adev->vcn.vcn_config[adev->vcn.num_vcn_inst] = >> - ip->revision & 0xc0; >> + >> ip->revision &= ~0xc0; >> if (adev->vcn.num_vcn_inst < >> AMDGPU_MAX_VCN_INSTANCES) { >> + adev->vcn.vcn_config[adev->vcn.num_vcn_inst] = >> + ip->revision & 0xc0; > > Hi, > I don't think that the patch is correct. > > Because of the "ip->revision &= ~0xc0" which is now above it, > "ip->revision & 0xc0" should now always be 0. > > Maybe both lines should be moved within the "if"? > > CJ Hi, Oh, really sorry for this, I just missed this line. you are right, I will send v2 soon. Thanks for your reminder! Su Hui > > > > > >> adev->vcn.num_vcn_inst++; >> adev->vcn.inst_mask |= >> (1U << ip->instance_number); >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c index 8e1cfc87122d..ba95526c3d45 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c @@ -1250,11 +1250,12 @@ static int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev) * 0b10 : encode is disabled * 0b01 : decode is disabled */ - adev->vcn.vcn_config[adev->vcn.num_vcn_inst] = - ip->revision & 0xc0; + ip->revision &= ~0xc0; if (adev->vcn.num_vcn_inst < AMDGPU_MAX_VCN_INSTANCES) { + adev->vcn.vcn_config[adev->vcn.num_vcn_inst] = + ip->revision & 0xc0; adev->vcn.num_vcn_inst++; adev->vcn.inst_mask |= (1U << ip->instance_number);
smatch error: drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1257 amdgpu_discovery_reg_base_init() error: testing array offset 'adev->vcn.num_vcn_inst' after use. change the assignment order to avoid buffer overflow. Fixes: c40bdfb2ffa4 ("drm/amdgpu: fix incorrect VCN revision in SRIOV") Signed-off-by: Su Hui <suhui@nfschina.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)