Re: [PATCH] drm/i915/kvmgt: avoid dereferencing a potentially null info pointer
On 2017.03.23 14:43:44 +0100, Frans Klaver wrote: > On Thu, Mar 23, 2017 at 1:22 PM, Colin Kingwrote: > > From: Colin Ian King > > > > info is being checked to see if it is a null pointer, however, vpgu is > > dereferencing info before this check, leading to a potential null > > pointer dereference. If info is null, then the error message being > > printed by macro gvt_vgpu_err and this requires vpgu to exist. We can > > use a null vpgu as the macro has a sanity check to see if vpgu is null, > > so this is OK. > > > > Detected with CoverityScan, CID#1420672 ("Dereference nefore null check") > > s,nefore,before, > > > > > > Fixes: 695fbc08d80f ("drm/i915/gvt: replace the gvt_err with gvt_vgpu_err") > > Signed-off-by: Colin Ian King > > --- > > drivers/gpu/drm/i915/gvt/kvmgt.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c > > b/drivers/gpu/drm/i915/gvt/kvmgt.c > > index 1ea3eb270de8..f8619a772c44 100644 > > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c > > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c > > @@ -1339,9 +1339,9 @@ static int kvmgt_guest_init(struct mdev_device *mdev) > > > > static bool kvmgt_guest_exit(struct kvmgt_guest_info *info) > > { > > - struct intel_vgpu *vgpu = info->vgpu; > > - > > if (!info) { > > + struct intel_vgpu *vgpu = NULL; > > + > > gvt_vgpu_err("kvmgt_guest_info invalid\n"); > > return false; > > } > > Does this mean the original gvt_err() macro is no longer there? > > And apparently gvt_vgpu_err is a macro that depends on the specifics > of its environment? Yikes. > The null check is redundant there, we can just remove that block and extra vgpu variable. -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature
Re: [PATCH] drm/i915/kvmgt: avoid dereferencing a potentially null info pointer
On 2017.03.23 14:43:44 +0100, Frans Klaver wrote: > On Thu, Mar 23, 2017 at 1:22 PM, Colin King wrote: > > From: Colin Ian King > > > > info is being checked to see if it is a null pointer, however, vpgu is > > dereferencing info before this check, leading to a potential null > > pointer dereference. If info is null, then the error message being > > printed by macro gvt_vgpu_err and this requires vpgu to exist. We can > > use a null vpgu as the macro has a sanity check to see if vpgu is null, > > so this is OK. > > > > Detected with CoverityScan, CID#1420672 ("Dereference nefore null check") > > s,nefore,before, > > > > > > Fixes: 695fbc08d80f ("drm/i915/gvt: replace the gvt_err with gvt_vgpu_err") > > Signed-off-by: Colin Ian King > > --- > > drivers/gpu/drm/i915/gvt/kvmgt.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c > > b/drivers/gpu/drm/i915/gvt/kvmgt.c > > index 1ea3eb270de8..f8619a772c44 100644 > > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c > > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c > > @@ -1339,9 +1339,9 @@ static int kvmgt_guest_init(struct mdev_device *mdev) > > > > static bool kvmgt_guest_exit(struct kvmgt_guest_info *info) > > { > > - struct intel_vgpu *vgpu = info->vgpu; > > - > > if (!info) { > > + struct intel_vgpu *vgpu = NULL; > > + > > gvt_vgpu_err("kvmgt_guest_info invalid\n"); > > return false; > > } > > Does this mean the original gvt_err() macro is no longer there? > > And apparently gvt_vgpu_err is a macro that depends on the specifics > of its environment? Yikes. > The null check is redundant there, we can just remove that block and extra vgpu variable. -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature
Re: [PATCH] drm/i915/kvmgt: avoid dereferencing a potentially null info pointer
On Thu, Mar 23, 2017 at 1:22 PM, Colin Kingwrote: > From: Colin Ian King > > info is being checked to see if it is a null pointer, however, vpgu is > dereferencing info before this check, leading to a potential null > pointer dereference. If info is null, then the error message being > printed by macro gvt_vgpu_err and this requires vpgu to exist. We can > use a null vpgu as the macro has a sanity check to see if vpgu is null, > so this is OK. > > Detected with CoverityScan, CID#1420672 ("Dereference nefore null check") s,nefore,before, > > Fixes: 695fbc08d80f ("drm/i915/gvt: replace the gvt_err with gvt_vgpu_err") > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c > b/drivers/gpu/drm/i915/gvt/kvmgt.c > index 1ea3eb270de8..f8619a772c44 100644 > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c > @@ -1339,9 +1339,9 @@ static int kvmgt_guest_init(struct mdev_device *mdev) > > static bool kvmgt_guest_exit(struct kvmgt_guest_info *info) > { > - struct intel_vgpu *vgpu = info->vgpu; > - > if (!info) { > + struct intel_vgpu *vgpu = NULL; > + > gvt_vgpu_err("kvmgt_guest_info invalid\n"); > return false; > } Does this mean the original gvt_err() macro is no longer there? And apparently gvt_vgpu_err is a macro that depends on the specifics of its environment? Yikes. Cheers, Frans
Re: [PATCH] drm/i915/kvmgt: avoid dereferencing a potentially null info pointer
On Thu, Mar 23, 2017 at 1:22 PM, Colin King wrote: > From: Colin Ian King > > info is being checked to see if it is a null pointer, however, vpgu is > dereferencing info before this check, leading to a potential null > pointer dereference. If info is null, then the error message being > printed by macro gvt_vgpu_err and this requires vpgu to exist. We can > use a null vpgu as the macro has a sanity check to see if vpgu is null, > so this is OK. > > Detected with CoverityScan, CID#1420672 ("Dereference nefore null check") s,nefore,before, > > Fixes: 695fbc08d80f ("drm/i915/gvt: replace the gvt_err with gvt_vgpu_err") > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/i915/gvt/kvmgt.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c > b/drivers/gpu/drm/i915/gvt/kvmgt.c > index 1ea3eb270de8..f8619a772c44 100644 > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c > @@ -1339,9 +1339,9 @@ static int kvmgt_guest_init(struct mdev_device *mdev) > > static bool kvmgt_guest_exit(struct kvmgt_guest_info *info) > { > - struct intel_vgpu *vgpu = info->vgpu; > - > if (!info) { > + struct intel_vgpu *vgpu = NULL; > + > gvt_vgpu_err("kvmgt_guest_info invalid\n"); > return false; > } Does this mean the original gvt_err() macro is no longer there? And apparently gvt_vgpu_err is a macro that depends on the specifics of its environment? Yikes. Cheers, Frans
Re: [PATCH] drm/i915/kvmgt: avoid dereferencing a potentially null info pointer
On Thu, Mar 23, 2017 at 12:22:30PM +, Colin King wrote: > From: Colin Ian King> > info is being checked to see if it is a null pointer, however, vpgu is > dereferencing info before this check, leading to a potential null > pointer dereference. If info is null, then the error message being > printed by macro gvt_vgpu_err and this requires vpgu to exist. We can > use a null vpgu as the macro has a sanity check to see if vpgu is null, > so this is OK. It is never NULL, it gets checked by its only caller. -Chris -- Chris Wilson, Intel Open Source Technology Centre
Re: [PATCH] drm/i915/kvmgt: avoid dereferencing a potentially null info pointer
On Thu, Mar 23, 2017 at 12:22:30PM +, Colin King wrote: > From: Colin Ian King > > info is being checked to see if it is a null pointer, however, vpgu is > dereferencing info before this check, leading to a potential null > pointer dereference. If info is null, then the error message being > printed by macro gvt_vgpu_err and this requires vpgu to exist. We can > use a null vpgu as the macro has a sanity check to see if vpgu is null, > so this is OK. It is never NULL, it gets checked by its only caller. -Chris -- Chris Wilson, Intel Open Source Technology Centre