Re: [PATCH 05/11] fs: add qemu_fw_cfg file system

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Sascha,

On 3/17/25 09:21, Sascha Hauer wrote:
> Hi Ahmad,
> 
> On Thu, Mar 13, 2025 at 11:17:22AM +0100, Ahmad Fatoum wrote:
>> +static int fw_cfg_fs_io(struct device *dev, struct file *f, void *buf,
>> +			 size_t insize, bool read)
>> +{
>> +	struct inode *inode = f->f_inode;
>> +	struct fw_cfg_fs_inode *node = inode_to_node(inode);
>> +	struct fw_cfg_fs_data *data = dev->priv;
>> +	int fd = data->fd;
>> +
>> +	if (node->buf) {
>> +		if (read)
>> +			memcpy(buf, node->buf + f->f_pos, insize);
>> +		else
>> +			memcpy(node->buf + f->f_pos, buf, insize);
>> +		return insize;
>> +	}
>> +
>> +	ioctl(fd, FW_CFG_SELECT, &(u16) { inode->i_ino });
>> +
>> +	if (read)
>> +		return pread(fd, buf, insize, f->f_pos);
>> +	else
>> +		return pwrite(fd, buf, insize, f->f_pos);
>> +}
>> +
>> +static int fw_cfg_fs_read(struct device *dev, struct file *f, void *buf,
>> +			   size_t insize)
>> +{
>> +	return fw_cfg_fs_io(dev, f, buf, insize, true);
>> +}
>> +
>> +static int fw_cfg_fs_write(struct device *dev, struct file *f, const void *buf,
>> +			    size_t insize)
>> +{
>> +	return fw_cfg_fs_io(dev, f, (void *)buf, insize, false);
>> +}
>> +
>> +static int fw_cfg_fw_truncate(struct device *dev, struct file *f, loff_t size)
>> +{
>> +	return 0;
>> +}
> 
> By returning 0 here you claim that the file can get an arbitrary size.
> 
> node->buf is a fixed size buffer. If I understand correctly then
> node->buf is only used for special files like "name", "raw".

Yes.

> However, writing to these files is not prohibited, consequently a
> cp /dev/ram0 /mnt/fw_cfg//by_key/37/key results in memory corrupions.

Ouch, I will revisit this.

> Writing data to a file I passed via --env truncates the file to the
> written data size, but the actual content of the file is unchanged. I
> still read back the original data. Is writing data supported?

I am not sure. I only wrote the special framebuffer file so far.
I will look into it.

Cheers,
Ahmad

> 
> Sascha
>