[bug report] drm/nouveau/secboot/gm20b: add secure boot support

[bug report] drm/nouveau/secboot/gm20b: add secure boot support

[Dan Carpenter]

Hello Alexandre Courbot,

The patch 923f1bd27bf1: "drm/nouveau/secboot/gm20b: add secure boot
support" from Feb 24, 2016, leads to the following static checker
warning:

	drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c:129 gm20b_secboot_new()
	warn: did you mean to set '*psb' instead of 'psb'

drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c
   103  int
   104  gm20b_secboot_new(struct nvkm_device *device, int index,
   105                    struct nvkm_secboot **psb)
   106  {
   107          int ret;
   108          struct gm200_secboot *gsb;
   109          struct nvkm_acr *acr;
   110  
   111          acr = acr_r352_new(BIT(NVKM_SECBOOT_FALCON_FECS) |
   112                             BIT(NVKM_SECBOOT_FALCON_PMU));
   113          if (IS_ERR(acr))
   114                  return PTR_ERR(acr);
   115          /* Support the initial GM20B firmware release without PMU */
   116          acr->optional_falcons = BIT(NVKM_SECBOOT_FALCON_PMU);
   117  
   118          gsb = kzalloc(sizeof(*gsb), GFP_KERNEL);
   119          if (!gsb) {
   120                  psb = NULL;

It's complaining about this.  We obviously did intend to set *psb = NULL
because the code is a no-op as it is now.  But shouldn't we just do it
at the start of the function so it's set for the other earlier return
as well?

   121                  return -ENOMEM;
   122          }
   123          *psb = &gsb->base;
   124  
   125          ret = nvkm_secboot_ctor(&gm20b_secboot, acr, device, index, &gsb->base);
   126          if (ret)
   127                  return ret;

And what about here, do we not want it to be NULL on this failure path?

I wasn't able to figure out how this code is called.  Normally Smatch is
able to figure out the call tree but I also wasn't able to figure it out
manually.  Could you point me where this function is called?

   128  
   129          return 0;
   130  }

This code is just cut and pasted and the other functions have this bug
as well.  See also:

drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gp102.c:170 gp102_secboot_new() warn: did you mean to set '*psb' instead of 'psb'
drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gp10b.c:74 gp10b_secboot_new() warn: did you mean to set '*psb' instead of 'psb'
drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm200.c:203 gm200_secboot_new() warn: did you mean to set '*psb' instead of 'psb'

regards,
dan carpenter
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau