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.
