| Message ID | 20200930090238.GA621698@mwanda (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: vchiq: silence an uninitialized variable warning | expand |
On Wed, Sep 30, 2020 at 11:02 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > Smatch complains that "userdata" can be passed to vchiq_bulk_transfer() > without being initialized. Smatch is correct, however, in that > situation the "userdata" is not used so it doesn't cause a problem. > Passing an uninitialized variable will trigger a UBSan warning at > runtime so this warning is worth silencing by setting "userdata" to > NULL. > > Fixes: a4367cd2b231 ("staging: vchiq: convert compat bulk transfer") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> The change looks fine, but I wonder if it's actually worse and the uninitialized pointer can end up getting copied back to user space in the completion. In either case, thanks for the fix! Acked-by: Arnd Bergmann <arnd@arndb.de>
On Wed, Sep 30, 2020 at 12:22:37PM +0200, Arnd Bergmann wrote: > On Wed, Sep 30, 2020 at 11:02 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > Smatch complains that "userdata" can be passed to vchiq_bulk_transfer() > > without being initialized. Smatch is correct, however, in that > > situation the "userdata" is not used so it doesn't cause a problem. > > Passing an uninitialized variable will trigger a UBSan warning at > > runtime so this warning is worth silencing by setting "userdata" to > > NULL. > > > > Fixes: a4367cd2b231 ("staging: vchiq: convert compat bulk transfer") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > The change looks fine, but I wonder if it's actually worse and the > uninitialized pointer can end up getting copied back to user space > in the completion. Ah... Wow. You're right. I think I really need to resend this with a more accurate commit message. regards, dan carpenter
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 590415561b73..71b962777da5 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -952,7 +952,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance, { struct vchiq_service *service; struct bulk_waiter_node *waiter = NULL; - void *userdata; + void *userdata = NULL; int status = 0; int ret;
Smatch complains that "userdata" can be passed to vchiq_bulk_transfer() without being initialized. Smatch is correct, however, in that situation the "userdata" is not used so it doesn't cause a problem. Passing an uninitialized variable will trigger a UBSan warning at runtime so this warning is worth silencing by setting "userdata" to NULL. Fixes: a4367cd2b231 ("staging: vchiq: convert compat bulk transfer") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)