[PATCH 1/1] sandbox: host bind must close file descriptor

[Heinrich Schuchardt]

On 2/1/21 9:44 PM, Simon Glass wrote:
> Hi Heinrich,
>
> On Sun, 31 Jan 2021 at 03:39, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> Each invocation of the 'host bind' command with a file name argument opens
>> a file descriptor. The next invocation of the 'host bind' command destroys
>> the block device but the file descriptor remains open. The same holds true
>> for the 'unbind blk' command.
>>
>> Close the file descriptor when unbinding the host block device.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>   drivers/block/sandbox.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/block/sandbox.c b/drivers/block/sandbox.c
>> index f57f690d3c..02992ac34f 100644
>> --- a/drivers/block/sandbox.c
>> +++ b/drivers/block/sandbox.c
>> @@ -230,6 +230,25 @@ int host_get_dev_err(int devnum, struct blk_desc **blk_devp)
>>   }
>>
>>   #ifdef CONFIG_BLK
>> +
>> +int sandbox_host_unbind(struct udevice *dev)
>> +{
>> +       struct host_block_dev *host_dev;
>> +
>> +       host_dev = dev_get_plat(dev);
>> +       if (host_dev) {
>> +               if (host_dev->fd) {
>> +                       os_close(host_dev->fd);
>> +                       host_dev->fd = 0;
>> +               } else {
>> +                       log_err("missing file descriptor\n");
>
> How does this happen?
>
>> +               }
>> +       } else {
>> +               log_err("missing platform data\n");
>
> I don't think this case can happen. See the .plat_auto below.

Both would only happen if there were a bug in the rest of the code.

Do you prefer assert_noisy() or should the messages be removed?

Best regards

Heinrich

>
>> +       }
>> +       return 0;
>> +}
>> +
>>   static const struct blk_ops sandbox_host_blk_ops = {
>>          .read   = host_block_read,
>>          .write  = host_block_write,
>> @@ -239,6 +258,7 @@ U_BOOT_DRIVER(sandbox_host_blk) = {
>>          .name           = "sandbox_host_blk",
>>          .id             = UCLASS_BLK,
>>          .ops            = &sandbox_host_blk_ops,
>> +       .unbind         = sandbox_host_unbind,
>>          .plat_auto      = sizeof(struct host_block_dev),
>>   };
>>   #else
>> --
>> 2.29.2
>>
>
> Regards,
> Simon
>