[Nouveau] [bug report] drm/nouveau/secboot/gm20b: add secure boot support
Dan Carpenter
dan.carpenter at oracle.com
Thu Apr 20 19:01:17 UTC 2017
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
More information about the Nouveau
mailing list