Re: [bug report] Drivers: hv: vmbus: Track decrypted status in vmbus_gpadl

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Sat, 2024-07-27 at 00:33 -0500, Dan Carpenter wrote:
> Commit 211f514ebf1e ("Drivers: hv: vmbus: Track decrypted status in
> vmbus_gpadl") from Mar 11, 2024 (linux-next), leads to the following
> Smatch static checker warning:
> 
>         drivers/hv/channel.c:870 vmbus_teardown_gpadl()
>         warn: assigning signed to unsigned: 'gpadl->decrypted = ret' 's32min-
> s32max'
> 
> drivers/hv/channel.c
>     860         list_del(&info->msglistentry);
>     861         spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock,
> flags);
>     862 
>     863         kfree(info);
>     864 
>     865         ret = set_memory_encrypted((unsigned long)gpadl->buffer,
>     866                                    PFN_UP(gpadl->size));
>     867         if (ret)
>     868                 pr_warn("Fail to set mem host visibility in GPADL
> teardown %d.\n", ret);
>     869 
> --> 870         gpadl->decrypted = ret;
> 
> ret is error codes but ->decrypted is bool.  So error codes mean decrypted is
> true.

If it fails, we need to assume that some of the buffer could still be decrypted,
so should have decrypted = true. Only if it is successful (ret == 0) should we
have decrypted = false.

So I think it is functionally correct. Should we have a cast for smatch's sake?
I would have thought there would be a lot of these patterns in the kernel.

If we unconditionally set decrypted = false here, I think we would be ok as
well. That would match the other similar cases better.