[PATCH] kernfs: implement custom llseek method to fix userspace regression

[PATCH] kernfs: implement custom llseek method to fix userspace regression

[Valentine Sinitsyn]

Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"),
mmapable sysfs binary attributes have started receiving their
f_mapping from the iomem pseudo filesystem, so that
CONFIG_IO_STRICT_DEVMEM is honored in sysfs (and procfs) as well
as in /dev/[k]mem.

This resulted in a userspace-visible regression: lseek(fd, 0, SEEK_END)
now returns zero regardless the real sysfs attribute size which stat()
reports. The reason is that kernfs files use generic_file_llseek()
implementation, which relies on f_mapping->host inode to get the file
size. As f_mapping is now redefined, f_mapping->host points to an
anonymous zero-sized iomem inode which has nothing to do with sysfs
attribute or kernfs file representing it. This being said, f_inode
remains valid, so stat() which uses it works correctly.

Fixes the regression by implementing a custom llseek fop for kernfs,
which uses an attribute's file inode to get the file size,
just as stat() does.

Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
---
 fs/kernfs/file.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 180906c36f51..6d81e0c981f3 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -903,6 +903,21 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait)
 	return ret;
 }
 
+static loff_t kernfs_fop_llseek(struct file *file, loff_t offset, int whence)
+{
+	/*
+	 * This is almost identical to generic_file_llseek() except it uses
+	 * cached inode value instead of f_mapping->host.
+	 * The reason is that, for PCI resources in sysfs the latter points to
+	 * iomem_inode whose size has nothing to do with the attribute's size.
+	 */
+	struct inode *inode = file_inode(file);
+
+	return generic_file_llseek_size(file, offset, whence,
+					inode->i_sb->s_maxbytes,
+					i_size_read(inode));
+}
+
 static void kernfs_notify_workfn(struct work_struct *work)
 {
 	struct kernfs_node *kn;
@@ -1005,7 +1020,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify);
 const struct file_operations kernfs_file_fops = {
 	.read_iter	= kernfs_fop_read_iter,
 	.write_iter	= kernfs_fop_write_iter,
-	.llseek		= generic_file_llseek,
+	.llseek		= kernfs_fop_llseek,
 	.mmap		= kernfs_fop_mmap,
 	.open		= kernfs_fop_open,
 	.release	= kernfs_fop_release,
-- 
2.34.1

RE: [PATCH] kernfs: implement custom llseek method to fix userspace regression

[Dan Williams]

Valentine Sinitsyn wrote:
> Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"),
> mmapable sysfs binary attributes have started receiving their
> f_mapping from the iomem pseudo filesystem, so that
> CONFIG_IO_STRICT_DEVMEM is honored in sysfs (and procfs) as well
> as in /dev/[k]mem.
> 
> This resulted in a userspace-visible regression: lseek(fd, 0, SEEK_END)
> now returns zero regardless the real sysfs attribute size which stat()
> reports. The reason is that kernfs files use generic_file_llseek()
> implementation, which relies on f_mapping->host inode to get the file
> size. As f_mapping is now redefined, f_mapping->host points to an
> anonymous zero-sized iomem inode which has nothing to do with sysfs
> attribute or kernfs file representing it. This being said, f_inode
> remains valid, so stat() which uses it works correctly.

Can you say a bit more about what userspace scenario regressed so that
others doing backports can make a judgement call on the severity?

> 
> Fixes the regression by implementing a custom llseek fop for kernfs,
> which uses an attribute's file inode to get the file size,
> just as stat() does.
> 
> Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
> ---
>  fs/kernfs/file.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
> index 180906c36f51..6d81e0c981f3 100644
> --- a/fs/kernfs/file.c
> +++ b/fs/kernfs/file.c
> @@ -903,6 +903,21 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait)
>  	return ret;
>  }
>  
> +static loff_t kernfs_fop_llseek(struct file *file, loff_t offset, int whence)
> +{
> +	/*
> +	 * This is almost identical to generic_file_llseek() except it uses
> +	 * cached inode value instead of f_mapping->host.
> +	 * The reason is that, for PCI resources in sysfs the latter points to
> +	 * iomem_inode whose size has nothing to do with the attribute's size.
> +	 */
> +	struct inode *inode = file_inode(file);

My only concern is whether there are any scenarios where this is not
appropriate. I.e. do a bit more work to define a kernfs_ops instance
specifically for overriding lseek() in this scenario.

> +
> +	return generic_file_llseek_size(file, offset, whence,
> +					inode->i_sb->s_maxbytes,
> +					i_size_read(inode));
> +}
> +
>  static void kernfs_notify_workfn(struct work_struct *work)
>  {
>  	struct kernfs_node *kn;
> @@ -1005,7 +1020,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify);
>  const struct file_operations kernfs_file_fops = {
>  	.read_iter	= kernfs_fop_read_iter,
>  	.write_iter	= kernfs_fop_write_iter,
> -	.llseek		= generic_file_llseek,
> +	.llseek		= kernfs_fop_llseek,
>  	.mmap		= kernfs_fop_mmap,
>  	.open		= kernfs_fop_open,
>  	.release	= kernfs_fop_release,
> -- 
> 2.34.1
> 

Re: [PATCH] kernfs: implement custom llseek method to fix userspace regression

[Valentin Sinitsyn]

On 15.08.2023 01:01, Dan Williams wrote:
> Valentine Sinitsyn wrote:
>> Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"),
>> mmapable sysfs binary attributes have started receiving their
>> f_mapping from the iomem pseudo filesystem, so that
>> CONFIG_IO_STRICT_DEVMEM is honored in sysfs (and procfs) as well
>> as in /dev/[k]mem.
>>
>> This resulted in a userspace-visible regression: lseek(fd, 0, SEEK_END)
>> now returns zero regardless the real sysfs attribute size which stat()
>> reports. The reason is that kernfs files use generic_file_llseek()
>> implementation, which relies on f_mapping->host inode to get the file
>> size. As f_mapping is now redefined, f_mapping->host points to an
>> anonymous zero-sized iomem inode which has nothing to do with sysfs
>> attribute or kernfs file representing it. This being said, f_inode
>> remains valid, so stat() which uses it works correctly.
> 
> Can you say a bit more about what userspace scenario regressed so that
> others doing backports can make a judgement call on the severity?

We've encountered this regression in the code which used lseek() to 
determine the size of PCI region. It was roughly equivalent to:

#define SYSFS_DEVICE_DIR "/sys/bus/pci/devices/<some id>/"

int fd = open(SYSFS_DEVICE_DIR "/resource0", O_RDWR);
off_t size = lseek(fd, 0, SEEK_END);
assert(size != 0)

Calling lseek() with whence argument other than SEEK_END and non-zero 
offset on this fd returns an error as the kernel considers it as seeking 
past EOF.

I'll add this explanation to v2 commit message.

> 
>>
>> Fixes the regression by implementing a custom llseek fop for kernfs,
>> which uses an attribute's file inode to get the file size,
>> just as stat() does.
>>
>> Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
>> ---
>>   fs/kernfs/file.c | 17 ++++++++++++++++-
>>   1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
>> index 180906c36f51..6d81e0c981f3 100644
>> --- a/fs/kernfs/file.c
>> +++ b/fs/kernfs/file.c
>> @@ -903,6 +903,21 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait)
>>   	return ret;
>>   }
>>   
>> +static loff_t kernfs_fop_llseek(struct file *file, loff_t offset, int whence)
>> +{
>> +	/*
>> +	 * This is almost identical to generic_file_llseek() except it uses
>> +	 * cached inode value instead of f_mapping->host.
>> +	 * The reason is that, for PCI resources in sysfs the latter points to
>> +	 * iomem_inode whose size has nothing to do with the attribute's size.
>> +	 */
>> +	struct inode *inode = file_inode(file);
> 
> My only concern is whether there are any scenarios where this is not
> appropriate. I.e. do a bit more work to define a kernfs_ops instance
> specifically for overriding lseek() in this scenario.

Not sure I'm getting you here: do you mean something like this?

struct inode *inode = is_f_mapping_redefined(file) ? file_inode(file) : 
file->f_mapping->host;

My understanding is file->f_inode should always be non-NULL and point to 
the inode corresponding the path of the opened file, so it should be 
safe to call regardless what f_mapping->host is. Do I miss anything?

Best,
Valentin

> 
>> +
>> +	return generic_file_llseek_size(file, offset, whence,
>> +					inode->i_sb->s_maxbytes,
>> +					i_size_read(inode));
>> +}
>> +
>>   static void kernfs_notify_workfn(struct work_struct *work)
>>   {
>>   	struct kernfs_node *kn;
>> @@ -1005,7 +1020,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify);
>>   const struct file_operations kernfs_file_fops = {
>>   	.read_iter	= kernfs_fop_read_iter,
>>   	.write_iter	= kernfs_fop_write_iter,
>> -	.llseek		= generic_file_llseek,
>> +	.llseek		= kernfs_fop_llseek,
>>   	.mmap		= kernfs_fop_mmap,
>>   	.open		= kernfs_fop_open,
>>   	.release	= kernfs_fop_release,
>> -- 
>> 2.34.1
>>
> 
> 

Re: [PATCH] kernfs: implement custom llseek method to fix userspace regression

[Dan Williams]

Valentin Sinitsyn wrote:
[..]
> > My only concern is whether there are any scenarios where this is not
> > appropriate. I.e. do a bit more work to define a kernfs_ops instance
> > specifically for overriding lseek() in this scenario.
> 
> Not sure I'm getting you here: do you mean something like this?
> 
> struct inode *inode = is_f_mapping_redefined(file) ? file_inode(file) : 
> file->f_mapping->host;

I meant something like the patch below (incomplete, but shows the idea).

> My understanding is file->f_inode should always be non-NULL and point to 
> the inode corresponding the path of the opened file, so it should be 
> safe to call regardless what f_mapping->host is. Do I miss anything?

That matches my understanding and I do not think you missed anything. At
the same time a comment about "PCI resources" is out of place in
fs/kernfs/file.c.

On the rare chance that someone down the line cares about the
difference, a more localized change like this lets this override be done
in generic terms (f_mapping override) without reference to PCI resource
specifics:

diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index a12ac0356c69..748804cd889f 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -167,6 +167,23 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of,
 	return battr->mmap(of->file, kobj, battr, vma);
 }
 
+static loff_t sysfs_kf_bin_llseek(struct kernfs_open_file *of, loff_t offset,
+				  int whence)
+{
+	struct bin_attribute *battr = of->kn->priv;
+	struct kobject *kobj = of->kn->parent->priv;
+
+	/* when mapping is overridden do not use it to lookup the inode */
+	if (battr->f_mapping) {
+		struct inode *file_inode(of->file);
+
+		return generic_file_llseek_size(of->file, offset, whence,
+						inode->i_sb->s_maxbytes,
+						i_size_read(inode));
+	}
+	return generic_file_llseek(of->file, offset, whence);
+}
+
 static int sysfs_kf_bin_open(struct kernfs_open_file *of)
 {
 	struct bin_attribute *battr = of->kn->priv;
@@ -249,6 +266,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = {
 	.write		= sysfs_kf_bin_write,
 	.mmap		= sysfs_kf_bin_mmap,
 	.open		= sysfs_kf_bin_open,
+	.llseek		= sysfs_kf_bin_llseek,
 };
 
 int sysfs_add_file_mode_ns(struct kernfs_node *parent,
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 73f5c120def8..9ed535930259 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -316,6 +316,7 @@ struct kernfs_ops {
 			 struct poll_table_struct *pt);
 
 	int (*mmap)(struct kernfs_open_file *of, struct vm_area_struct *vma);
+	loff_t (*llseek)(struct kernfs_open_file *of, loff_t offset, int whence);
 };
 
 /*

Re: [PATCH] kernfs: implement custom llseek method to fix userspace regression

[Valentin Sinitsyn]

On 15.08.2023 20:48, Dan Williams wrote:
> Valentin Sinitsyn wrote:
> [..]
>>> My only concern is whether there are any scenarios where this is not
>>> appropriate. I.e. do a bit more work to define a kernfs_ops instance
>>> specifically for overriding lseek() in this scenario.
>>
>> Not sure I'm getting you here: do you mean something like this?
>>
>> struct inode *inode = is_f_mapping_redefined(file) ? file_inode(file) :
>> file->f_mapping->host;
> 
> I meant something like the patch below (incomplete, but shows the idea).
Understood, thanks. I believe the change can be localized even further, 
up to the point where battr->f_mapping is initialized with 
iomem_get_mapping, as it is the only override known to break 
generic_file_llseek().

I'll send the updated patch in a few days.

Best,
Valentin

[RESEND PATCH v2 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

[Valentine Sinitsyn]

As of now, seeking in sysfs files is handled by generic_file_llseek().
There are situations where one may want to customize seeking logic:

- Many sysfs entries are fixed files while generic_file_llseek() accepts
  past-the-end positions. Not only being useless by itself, this
  also means a bug in userspace code will trigger not at lseek(), but at
  some later point making debugging harder.
- generic_file_llseek() relies on f_mapping->host to get the file size
  which might not be correct for all sysfs entries.
  See commit 636b21b50152 ("PCI: Revoke mappings like devmem") as an example.

Implement llseek method to override this behavior at sysfs attribute
level. The method is optional, and if it is absent,
generic_file_llseek() is called to preserve backwards compatibility.

Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
---
Looks like the patch didn't get to lkml, resending

 fs/kernfs/file.c       | 14 +++++++++++++-
 fs/sysfs/file.c        | 13 +++++++++++++
 include/linux/kernfs.h |  1 +
 include/linux/sysfs.h  |  2 ++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 180906c36f51..6166bf74d5b8 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -903,6 +903,18 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait)
 	return ret;
 }

+static loff_t kernfs_fop_llseek(struct file *file, loff_t offset, int whence)
+{
+	struct kernfs_open_file *of = kernfs_of(file);
+	const struct kernfs_ops *ops;
+
+	ops = kernfs_ops(of->kn);
+	if (ops->llseek)
+		return ops->llseek(of, offset, whence);
+	else
+		return generic_file_llseek(file, offset, whence);
+}
+
 static void kernfs_notify_workfn(struct work_struct *work)
 {
 	struct kernfs_node *kn;
@@ -1005,7 +1017,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify);
 const struct file_operations kernfs_file_fops = {
 	.read_iter	= kernfs_fop_read_iter,
 	.write_iter	= kernfs_fop_write_iter,
-	.llseek		= generic_file_llseek,
+	.llseek		= kernfs_fop_llseek,
 	.mmap		= kernfs_fop_mmap,
 	.open		= kernfs_fop_open,
 	.release	= kernfs_fop_release,
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index a12ac0356c69..6b7652fb8050 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -167,6 +167,18 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of,
 	return battr->mmap(of->file, kobj, battr, vma);
 }
 
+static loff_t sysfs_kf_bin_llseek(struct kernfs_open_file *of, loff_t offset,
+				  int whence)
+{
+	struct bin_attribute *battr = of->kn->priv;
+	struct kobject *kobj = of->kn->parent->priv;
+
+	if (battr->llseek)
+		return battr->llseek(of->file, kobj, battr, offset, whence);
+	else
+		return generic_file_llseek(of->file, offset, whence);
+}
+
 static int sysfs_kf_bin_open(struct kernfs_open_file *of)
 {
 	struct bin_attribute *battr = of->kn->priv;
@@ -249,6 +261,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = {
 	.write		= sysfs_kf_bin_write,
 	.mmap		= sysfs_kf_bin_mmap,
 	.open		= sysfs_kf_bin_open,
+	.llseek		= sysfs_kf_bin_llseek,
 };
 
 int sysfs_add_file_mode_ns(struct kernfs_node *parent,
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 73f5c120def8..9ed535930259 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -316,6 +316,7 @@ struct kernfs_ops {
 			 struct poll_table_struct *pt);
 
 	int (*mmap)(struct kernfs_open_file *of, struct vm_area_struct *vma);
+	loff_t (*llseek)(struct kernfs_open_file *of, loff_t offset, int whence);
 };
 
 /*
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index fd3fe5c8c17f..b717a70219f6 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -181,6 +181,8 @@ struct bin_attribute {
 			char *, loff_t, size_t);
 	ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
 			 char *, loff_t, size_t);
+	loff_t (*llseek)(struct file *, struct kobject *, struct bin_attribute *,
+			 loff_t, int);
 	int (*mmap)(struct file *, struct kobject *, struct bin_attribute *attr,
 		    struct vm_area_struct *vma);
 };
-- 
2.34.1

[RESEND PATCH v2 2/2] PCI: implement custom llseek method for PCI resource entries in sysfs

[Valentine Sinitsyn]

Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"), mmapable
sysfs entries have started to receive their f_mapping from the iomem
pseudo filesystem, so that CONFIG_IO_STRICT_DEVMEM is honored in sysfs
(and procfs) as well as in /dev/[k]mem.

This resulted in a userspace-visible regression:

1. Open a sysfs PCI resource file (eg. /sys/bus/pci/devices/*/resource0)
2. Use lseek(fd, 0, SEEK_END) to determine its size

Expected result: a PCI region size is returned.
Actual result: 0 is returned.

The reason is that PCI resource files residing in sysfs use
generic_file_llseek(), which relies on f_mapping->host inode to get the
file size. As f_mapping is now redefined, f_mapping->host points to an
anonymous zero-sized iomem_inode which has nothing to do with sysfs file
in question.

Implement a custom llseek method for sysfs PCI resources, which is
almost the same as proc_bus_pci_lseek() used for procfs entries.

This makes sysfs and procfs entries consistent with regards to seeking,
but also introduces userspace-visible changes to seeking PCI resources
in sysfs:

- SEEK_DATA and SEEK_HOLE are no longer supported;
- Seeking past the end of the file is prohibited while previously
  offsets up to MAX_NON_LFS were accepted (reading from these offsets
  was always invalid).

Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
---
Looks like the patch didn't get to lkml, resending

 drivers/pci/pci-sysfs.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index ab32a91f287b..42eaeb8d4a4f 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -967,6 +967,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_io->attr.mode = 0600;
 	b->legacy_io->read = pci_read_legacy_io;
 	b->legacy_io->write = pci_write_legacy_io;
+	/* See pci_create_attr() for motivation */
+	b->legacy_io->llseek = pci_llseek_resource;
 	b->legacy_io->mmap = pci_mmap_legacy_io;
 	b->legacy_io->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_io);
@@ -981,6 +983,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_mem->size = 1024*1024;
 	b->legacy_mem->attr.mode = 0600;
 	b->legacy_mem->mmap = pci_mmap_legacy_mem;
+	/* See pci_create_attr() for motivation */
+	b->legacy_io->llseek = pci_llseek_resource;
 	b->legacy_mem->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_mem);
 	error = device_create_bin_file(&b->dev, b->legacy_mem);
@@ -1138,6 +1142,14 @@ static ssize_t pci_write_resource_io(struct file *filp, struct kobject *kobj,
 	return pci_resource_io(filp, kobj, attr, buf, off, count, true);
 }
 
+static loff_t pci_llseek_resource(struct file *filep,
+				  struct kobject *kobj __always_unused,
+				  struct bin_attribute *attr,
+				  loff_t offset, int whence)
+{
+	return fixed_size_llseek(filep, offset, whence, attr->size);
+}
+
 /**
  * pci_remove_resource_files - cleanup resource files
  * @pdev: dev to cleanup
@@ -1195,8 +1207,15 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
 			res_attr->mmap = pci_mmap_resource_uc;
 		}
 	}
-	if (res_attr->mmap)
+	if (res_attr->mmap) {
 		res_attr->f_mapping = iomem_get_mapping;
+		/*
+		 * generic_file_llseek() consults f_mapping->host to determine
+		 * the file size. As iomem_inode knows nothing about the
+		 * attribute, it's not going to work, so override it as well.
+		 */
+		res_attr->llseek = pci_llseek_resource;
+	}
 	res_attr->attr.name = res_attr_name;
 	res_attr->attr.mode = 0600;
 	res_attr->size = pci_resource_len(pdev, num);
-- 
2.34.1

Re: [RESEND PATCH v2 2/2] PCI: implement custom llseek method for PCI resource entries in sysfs

[Bjorn Helgaas]

In subject, to match history:

  PCI: Implement custom llseek for sysfs resource entries

On Mon, Aug 21, 2023 at 12:29:56PM +0500, Valentine Sinitsyn wrote:
> Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"), mmapable
> sysfs entries have started to receive their f_mapping from the iomem
> pseudo filesystem, so that CONFIG_IO_STRICT_DEVMEM is honored in sysfs
> (and procfs) as well as in /dev/[k]mem.

s/mmapable/mmappable/ (there's precedent for both, but by analogy with
"mappable", I think "mmappable" makes more sense)

> This resulted in a userspace-visible regression:
> 
> 1. Open a sysfs PCI resource file (eg. /sys/bus/pci/devices/*/resource0)
> 2. Use lseek(fd, 0, SEEK_END) to determine its size
> 
> Expected result: a PCI region size is returned.
> Actual result: 0 is returned.
> 
> The reason is that PCI resource files residing in sysfs use
> generic_file_llseek(), which relies on f_mapping->host inode to get the
> file size. As f_mapping is now redefined, f_mapping->host points to an
> anonymous zero-sized iomem_inode which has nothing to do with sysfs file
> in question.
> 
> Implement a custom llseek method for sysfs PCI resources, which is
> almost the same as proc_bus_pci_lseek() used for procfs entries.
> 
> This makes sysfs and procfs entries consistent with regards to seeking,
> but also introduces userspace-visible changes to seeking PCI resources
> in sysfs:
> 
> - SEEK_DATA and SEEK_HOLE are no longer supported;
> - Seeking past the end of the file is prohibited while previously
>   offsets up to MAX_NON_LFS were accepted (reading from these offsets
>   was always invalid).
> 
> Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>

It'd be nice but not essential to tweak commit log as above.

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
> Looks like the patch didn't get to lkml, resending
> 
>  drivers/pci/pci-sysfs.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index ab32a91f287b..42eaeb8d4a4f 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -967,6 +967,8 @@ void pci_create_legacy_files(struct pci_bus *b)
>  	b->legacy_io->attr.mode = 0600;
>  	b->legacy_io->read = pci_read_legacy_io;
>  	b->legacy_io->write = pci_write_legacy_io;
> +	/* See pci_create_attr() for motivation */
> +	b->legacy_io->llseek = pci_llseek_resource;
>  	b->legacy_io->mmap = pci_mmap_legacy_io;
>  	b->legacy_io->f_mapping = iomem_get_mapping;
>  	pci_adjust_legacy_attr(b, pci_mmap_io);
> @@ -981,6 +983,8 @@ void pci_create_legacy_files(struct pci_bus *b)
>  	b->legacy_mem->size = 1024*1024;
>  	b->legacy_mem->attr.mode = 0600;
>  	b->legacy_mem->mmap = pci_mmap_legacy_mem;
> +	/* See pci_create_attr() for motivation */
> +	b->legacy_io->llseek = pci_llseek_resource;
>  	b->legacy_mem->f_mapping = iomem_get_mapping;
>  	pci_adjust_legacy_attr(b, pci_mmap_mem);
>  	error = device_create_bin_file(&b->dev, b->legacy_mem);
> @@ -1138,6 +1142,14 @@ static ssize_t pci_write_resource_io(struct file *filp, struct kobject *kobj,
>  	return pci_resource_io(filp, kobj, attr, buf, off, count, true);
>  }
>  
> +static loff_t pci_llseek_resource(struct file *filep,
> +				  struct kobject *kobj __always_unused,
> +				  struct bin_attribute *attr,
> +				  loff_t offset, int whence)
> +{
> +	return fixed_size_llseek(filep, offset, whence, attr->size);
> +}
> +
>  /**
>   * pci_remove_resource_files - cleanup resource files
>   * @pdev: dev to cleanup
> @@ -1195,8 +1207,15 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
>  			res_attr->mmap = pci_mmap_resource_uc;
>  		}
>  	}
> -	if (res_attr->mmap)
> +	if (res_attr->mmap) {
>  		res_attr->f_mapping = iomem_get_mapping;
> +		/*
> +		 * generic_file_llseek() consults f_mapping->host to determine
> +		 * the file size. As iomem_inode knows nothing about the
> +		 * attribute, it's not going to work, so override it as well.
> +		 */
> +		res_attr->llseek = pci_llseek_resource;
> +	}
>  	res_attr->attr.name = res_attr_name;
>  	res_attr->attr.mode = 0600;
>  	res_attr->size = pci_resource_len(pdev, num);
> -- 
> 2.34.1
> 

Re: [RESEND PATCH v2 2/2] PCI: implement custom llseek method for PCI resource entries in sysfs

[kernel test robot]

Hi Valentine,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.5-rc7 next-20230821]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Valentine-Sinitsyn/PCI-implement-custom-llseek-method-for-PCI-resource-entries-in-sysfs/20230821-163118
base:   linus/master
patch link:    https://lore.kernel.org/r/20230821072956.114193-2-valesini%40yandex-team.ru
patch subject: [RESEND PATCH v2 2/2] PCI: implement custom llseek method for PCI resource entries in sysfs
config: powerpc-randconfig-r024-20230821 (https://download.01.org/0day-ci/archive/20230822/202308220648.wcmc5jWq-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230822/202308220648.wcmc5jWq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308220648.wcmc5jWq-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pci/pci-sysfs.c:936:13: warning: no previous prototype for 'pci_adjust_legacy_attr' [-Wmissing-prototypes]
     936 | void __weak pci_adjust_legacy_attr(struct pci_bus *b,
         |             ^~~~~~~~~~~~~~~~~~~~~~
   drivers/pci/pci-sysfs.c: In function 'pci_create_legacy_files':
>> drivers/pci/pci-sysfs.c:971:32: error: 'pci_llseek_resource' undeclared (first use in this function); did you mean 'pci_claim_resource'?
     971 |         b->legacy_io->llseek = pci_llseek_resource;
         |                                ^~~~~~~~~~~~~~~~~~~
         |                                pci_claim_resource
   drivers/pci/pci-sysfs.c:971:32: note: each undeclared identifier is reported only once for each function it appears in


vim +971 drivers/pci/pci-sysfs.c

   940	
   941	/**
   942	 * pci_create_legacy_files - create legacy I/O port and memory files
   943	 * @b: bus to create files under
   944	 *
   945	 * Some platforms allow access to legacy I/O port and ISA memory space on
   946	 * a per-bus basis.  This routine creates the files and ties them into
   947	 * their associated read, write and mmap files from pci-sysfs.c
   948	 *
   949	 * On error unwind, but don't propagate the error to the caller
   950	 * as it is ok to set up the PCI bus without these files.
   951	 */
   952	void pci_create_legacy_files(struct pci_bus *b)
   953	{
   954		int error;
   955	
   956		if (!sysfs_initialized)
   957			return;
   958	
   959		b->legacy_io = kcalloc(2, sizeof(struct bin_attribute),
   960				       GFP_ATOMIC);
   961		if (!b->legacy_io)
   962			goto kzalloc_err;
   963	
   964		sysfs_bin_attr_init(b->legacy_io);
   965		b->legacy_io->attr.name = "legacy_io";
   966		b->legacy_io->size = 0xffff;
   967		b->legacy_io->attr.mode = 0600;
   968		b->legacy_io->read = pci_read_legacy_io;
   969		b->legacy_io->write = pci_write_legacy_io;
   970		/* See pci_create_attr() for motivation */
 > 971		b->legacy_io->llseek = pci_llseek_resource;
   972		b->legacy_io->mmap = pci_mmap_legacy_io;
   973		b->legacy_io->f_mapping = iomem_get_mapping;
   974		pci_adjust_legacy_attr(b, pci_mmap_io);
   975		error = device_create_bin_file(&b->dev, b->legacy_io);
   976		if (error)
   977			goto legacy_io_err;
   978	
   979		/* Allocated above after the legacy_io struct */
   980		b->legacy_mem = b->legacy_io + 1;
   981		sysfs_bin_attr_init(b->legacy_mem);
   982		b->legacy_mem->attr.name = "legacy_mem";
   983		b->legacy_mem->size = 1024*1024;
   984		b->legacy_mem->attr.mode = 0600;
   985		b->legacy_mem->mmap = pci_mmap_legacy_mem;
   986		/* See pci_create_attr() for motivation */
   987		b->legacy_io->llseek = pci_llseek_resource;
   988		b->legacy_mem->f_mapping = iomem_get_mapping;
   989		pci_adjust_legacy_attr(b, pci_mmap_mem);
   990		error = device_create_bin_file(&b->dev, b->legacy_mem);
   991		if (error)
   992			goto legacy_mem_err;
   993	
   994		return;
   995	
   996	legacy_mem_err:
   997		device_remove_bin_file(&b->dev, b->legacy_io);
   998	legacy_io_err:
   999		kfree(b->legacy_io);
  1000		b->legacy_io = NULL;
  1001	kzalloc_err:
  1002		dev_warn(&b->dev, "could not create legacy I/O port and ISA memory resources in sysfs\n");
  1003	}
  1004	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v3 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

[Valentine Sinitsyn]

As of now, seeking in sysfs files is handled by generic_file_llseek().
There are situations where one may want to customize seeking logic:

- Many sysfs entries are fixed files while generic_file_llseek() accepts
  past-the-end positions. Not only being useless by itself, this
  also means a bug in userspace code will trigger not at lseek(), but at
  some later point making debugging harder.
- generic_file_llseek() relies on f_mapping->host to get the file size
  which might not be correct for all sysfs entries.
  See commit 636b21b50152 ("PCI: Revoke mappings like devmem") as an example.

Implement llseek method to override this behavior at sysfs attribute
level. The method is optional, and if it is absent,
generic_file_llseek() is called to preserve backwards compatibility.

Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
---
v3:
        - Grammar fixes
        - Add base-patch: and prerequisite-patch-id: to make kernel test
          robot happy
v2:
        - Add infrastructure to customize llseek per sysfs entry type
        - Override llseek for PCI sysfs entries with fixed_file_llseek()

 fs/kernfs/file.c       | 14 +++++++++++++-
 fs/sysfs/file.c        | 13 +++++++++++++
 include/linux/kernfs.h |  1 +
 include/linux/sysfs.h  |  2 ++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 180906c36f51..6166bf74d5b8 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -903,6 +903,18 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait)
 	return ret;
 }
 
+static loff_t kernfs_fop_llseek(struct file *file, loff_t offset, int whence)
+{
+	struct kernfs_open_file *of = kernfs_of(file);
+	const struct kernfs_ops *ops;
+
+	ops = kernfs_ops(of->kn);
+	if (ops->llseek)
+		return ops->llseek(of, offset, whence);
+	else
+		return generic_file_llseek(file, offset, whence);
+}
+
 static void kernfs_notify_workfn(struct work_struct *work)
 {
 	struct kernfs_node *kn;
@@ -1005,7 +1017,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify);
 const struct file_operations kernfs_file_fops = {
 	.read_iter	= kernfs_fop_read_iter,
 	.write_iter	= kernfs_fop_write_iter,
-	.llseek		= generic_file_llseek,
+	.llseek		= kernfs_fop_llseek,
 	.mmap		= kernfs_fop_mmap,
 	.open		= kernfs_fop_open,
 	.release	= kernfs_fop_release,
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index a12ac0356c69..6b7652fb8050 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -167,6 +167,18 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of,
 	return battr->mmap(of->file, kobj, battr, vma);
 }
 
+static loff_t sysfs_kf_bin_llseek(struct kernfs_open_file *of, loff_t offset,
+				  int whence)
+{
+	struct bin_attribute *battr = of->kn->priv;
+	struct kobject *kobj = of->kn->parent->priv;
+
+	if (battr->llseek)
+		return battr->llseek(of->file, kobj, battr, offset, whence);
+	else
+		return generic_file_llseek(of->file, offset, whence);
+}
+
 static int sysfs_kf_bin_open(struct kernfs_open_file *of)
 {
 	struct bin_attribute *battr = of->kn->priv;
@@ -249,6 +261,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = {
 	.write		= sysfs_kf_bin_write,
 	.mmap		= sysfs_kf_bin_mmap,
 	.open		= sysfs_kf_bin_open,
+	.llseek		= sysfs_kf_bin_llseek,
 };
 
 int sysfs_add_file_mode_ns(struct kernfs_node *parent,
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 73f5c120def8..9ed535930259 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -316,6 +316,7 @@ struct kernfs_ops {
 			 struct poll_table_struct *pt);
 
 	int (*mmap)(struct kernfs_open_file *of, struct vm_area_struct *vma);
+	loff_t (*llseek)(struct kernfs_open_file *of, loff_t offset, int whence);
 };
 
 /*
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index fd3fe5c8c17f..b717a70219f6 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -181,6 +181,8 @@ struct bin_attribute {
 			char *, loff_t, size_t);
 	ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
 			 char *, loff_t, size_t);
+	loff_t (*llseek)(struct file *, struct kobject *, struct bin_attribute *,
+			 loff_t, int);
 	int (*mmap)(struct file *, struct kobject *, struct bin_attribute *attr,
 		    struct vm_area_struct *vma);
 };

base-commit: f7757129e3dea336c407551c98f50057c22bb266
-- 
2.34.1

[PATCH v3 2/2] PCI: Implement custom llseek for sysfs resource entries

[Valentine Sinitsyn]

Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"), mmappable
sysfs entries have started to receive their f_mapping from the iomem
pseudo filesystem, so that CONFIG_IO_STRICT_DEVMEM is honored in sysfs
(and procfs) as well as in /dev/[k]mem.

This resulted in a userspace-visible regression:

1. Open a sysfs PCI resource file (eg. /sys/bus/pci/devices/*/resource0)
2. Use lseek(fd, 0, SEEK_END) to determine its size

Expected result: a PCI region size is returned.
Actual result: 0 is returned.

The reason is that PCI resource files residing in sysfs use
generic_file_llseek(), which relies on f_mapping->host inode to get the
file size. As f_mapping is now redefined, f_mapping->host points to an
anonymous zero-sized iomem_inode which has nothing to do with sysfs file
in question.

Implement a custom llseek method for sysfs PCI resources, which is
almost the same as proc_bus_pci_lseek() used for procfs entries.

This makes sysfs and procfs entries consistent with regards to seeking,
but also introduces userspace-visible changes to seeking PCI resources
in sysfs:

- SEEK_DATA and SEEK_HOLE are no longer supported;
- Seeking past the end of the file is prohibited while previously
  offsets up to MAX_NON_LFS were accepted (reading from these offsets
  was always invalid).

Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci-sysfs.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index ab32a91f287b..42eaeb8d4a4f 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -967,6 +967,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_io->attr.mode = 0600;
 	b->legacy_io->read = pci_read_legacy_io;
 	b->legacy_io->write = pci_write_legacy_io;
+	/* See pci_create_attr() for motivation */
+	b->legacy_io->llseek = pci_llseek_resource;
 	b->legacy_io->mmap = pci_mmap_legacy_io;
 	b->legacy_io->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_io);
@@ -981,6 +983,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_mem->size = 1024*1024;
 	b->legacy_mem->attr.mode = 0600;
 	b->legacy_mem->mmap = pci_mmap_legacy_mem;
+	/* See pci_create_attr() for motivation */
+	b->legacy_io->llseek = pci_llseek_resource;
 	b->legacy_mem->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_mem);
 	error = device_create_bin_file(&b->dev, b->legacy_mem);
@@ -1138,6 +1142,14 @@ static ssize_t pci_write_resource_io(struct file *filp, struct kobject *kobj,
 	return pci_resource_io(filp, kobj, attr, buf, off, count, true);
 }
 
+static loff_t pci_llseek_resource(struct file *filep,
+				  struct kobject *kobj __always_unused,
+				  struct bin_attribute *attr,
+				  loff_t offset, int whence)
+{
+	return fixed_size_llseek(filep, offset, whence, attr->size);
+}
+
 /**
  * pci_remove_resource_files - cleanup resource files
  * @pdev: dev to cleanup
@@ -1195,8 +1207,15 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
 			res_attr->mmap = pci_mmap_resource_uc;
 		}
 	}
-	if (res_attr->mmap)
+	if (res_attr->mmap) {
 		res_attr->f_mapping = iomem_get_mapping;
+		/*
+		 * generic_file_llseek() consults f_mapping->host to determine
+		 * the file size. As iomem_inode knows nothing about the
+		 * attribute, it's not going to work, so override it as well.
+		 */
+		res_attr->llseek = pci_llseek_resource;
+	}
 	res_attr->attr.name = res_attr_name;
 	res_attr->attr.mode = 0600;
 	res_attr->size = pci_resource_len(pdev, num);

prerequisite-patch-id: 955ac8520d17a30dc71166fa7f1d25c09cbab46e
-- 
2.34.1

Re: [RESEND PATCH v2 2/2] PCI: implement custom llseek method for PCI resource entries in sysfs

[Valentin Sinitsyn]

On 22.08.2023 04:15, kernel test robot wrote:
> Hi Valentine,
> 
> kernel test robot noticed the following build errors:
Just realized that v3 doesn't really address this, will send the updated 
patch shortly.

Best,
Valentin

[PATCH v4 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

[Valentine Sinitsyn]

As of now, seeking in sysfs files is handled by generic_file_llseek().
There are situations where one may want to customize seeking logic:

- Many sysfs entries are fixed files while generic_file_llseek() accepts
  past-the-end positions. Not only being useless by itself, this
  also means a bug in userspace code will trigger not at lseek(), but at
  some later point making debugging harder.
- generic_file_llseek() relies on f_mapping->host to get the file size
  which might not be correct for all sysfs entries.
  See commit 636b21b50152 ("PCI: Revoke mappings like devmem") as an example.

Implement llseek method to override this behavior at sysfs attribute
level. The method is optional, and if it is absent,
generic_file_llseek() is called to preserve backwards compatibility.

Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
---
v4:
        - Fix builds which #define HAVE_PCI_LEGACY (e.g. PowerPC)
v3:
        - Grammar fixes
        - Add base-patch: and prerequisite-patch-id: to make kernel test
          robot happy
v2:
        - Add infrastructure to customize llseek per sysfs entry type
        - Override llseek for PCI sysfs entries with fixed_file_llseek()

 fs/kernfs/file.c       | 14 +++++++++++++-
 fs/sysfs/file.c        | 13 +++++++++++++
 include/linux/kernfs.h |  1 +
 include/linux/sysfs.h  |  2 ++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 180906c36f51..6166bf74d5b8 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -903,6 +903,18 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait)
 	return ret;
 }
 
+static loff_t kernfs_fop_llseek(struct file *file, loff_t offset, int whence)
+{
+	struct kernfs_open_file *of = kernfs_of(file);
+	const struct kernfs_ops *ops;
+
+	ops = kernfs_ops(of->kn);
+	if (ops->llseek)
+		return ops->llseek(of, offset, whence);
+	else
+		return generic_file_llseek(file, offset, whence);
+}
+
 static void kernfs_notify_workfn(struct work_struct *work)
 {
 	struct kernfs_node *kn;
@@ -1005,7 +1017,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify);
 const struct file_operations kernfs_file_fops = {
 	.read_iter	= kernfs_fop_read_iter,
 	.write_iter	= kernfs_fop_write_iter,
-	.llseek		= generic_file_llseek,
+	.llseek		= kernfs_fop_llseek,
 	.mmap		= kernfs_fop_mmap,
 	.open		= kernfs_fop_open,
 	.release	= kernfs_fop_release,
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index a12ac0356c69..6b7652fb8050 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -167,6 +167,18 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of,
 	return battr->mmap(of->file, kobj, battr, vma);
 }
 
+static loff_t sysfs_kf_bin_llseek(struct kernfs_open_file *of, loff_t offset,
+				  int whence)
+{
+	struct bin_attribute *battr = of->kn->priv;
+	struct kobject *kobj = of->kn->parent->priv;
+
+	if (battr->llseek)
+		return battr->llseek(of->file, kobj, battr, offset, whence);
+	else
+		return generic_file_llseek(of->file, offset, whence);
+}
+
 static int sysfs_kf_bin_open(struct kernfs_open_file *of)
 {
 	struct bin_attribute *battr = of->kn->priv;
@@ -249,6 +261,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = {
 	.write		= sysfs_kf_bin_write,
 	.mmap		= sysfs_kf_bin_mmap,
 	.open		= sysfs_kf_bin_open,
+	.llseek		= sysfs_kf_bin_llseek,
 };
 
 int sysfs_add_file_mode_ns(struct kernfs_node *parent,
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 73f5c120def8..9ed535930259 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -316,6 +316,7 @@ struct kernfs_ops {
 			 struct poll_table_struct *pt);
 
 	int (*mmap)(struct kernfs_open_file *of, struct vm_area_struct *vma);
+	loff_t (*llseek)(struct kernfs_open_file *of, loff_t offset, int whence);
 };
 
 /*
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index fd3fe5c8c17f..b717a70219f6 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -181,6 +181,8 @@ struct bin_attribute {
 			char *, loff_t, size_t);
 	ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
 			 char *, loff_t, size_t);
+	loff_t (*llseek)(struct file *, struct kobject *, struct bin_attribute *,
+			 loff_t, int);
 	int (*mmap)(struct file *, struct kobject *, struct bin_attribute *attr,
 		    struct vm_area_struct *vma);
 };

base-commit: f7757129e3dea336c407551c98f50057c22bb266
-- 
2.34.1

[PATCH v4 2/2] PCI: Implement custom llseek for sysfs resource entries

[Valentine Sinitsyn]

Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"), mmappable
sysfs entries have started to receive their f_mapping from the iomem
pseudo filesystem, so that CONFIG_IO_STRICT_DEVMEM is honored in sysfs
(and procfs) as well as in /dev/[k]mem.

This resulted in a userspace-visible regression:

1. Open a sysfs PCI resource file (eg. /sys/bus/pci/devices/*/resource0)
2. Use lseek(fd, 0, SEEK_END) to determine its size

Expected result: a PCI region size is returned.
Actual result: 0 is returned.

The reason is that PCI resource files residing in sysfs use
generic_file_llseek(), which relies on f_mapping->host inode to get the
file size. As f_mapping is now redefined, f_mapping->host points to an
anonymous zero-sized iomem_inode which has nothing to do with sysfs file
in question.

Implement a custom llseek method for sysfs PCI resources, which is
almost the same as proc_bus_pci_lseek() used for procfs entries.

This makes sysfs and procfs entries consistent with regards to seeking,
but also introduces userspace-visible changes to seeking PCI resources
in sysfs:

- SEEK_DATA and SEEK_HOLE are no longer supported;
- Seeking past the end of the file is prohibited while previously
  offsets up to MAX_NON_LFS were accepted (reading from these offsets
  was always invalid).

Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci-sysfs.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index ab32a91f287b..2b9d93f11dd1 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -836,6 +836,12 @@ static const struct attribute_group pci_dev_config_attr_group = {
 };
 
 #ifdef HAVE_PCI_LEGACY
+
+static loff_t pci_llseek_resource(struct file *filep,
+				  struct kobject *kobj,
+				  struct bin_attribute *attr,
+				  loff_t offset, int whence);
+
 /**
  * pci_read_legacy_io - read byte(s) from legacy I/O port space
  * @filp: open sysfs file
@@ -967,6 +973,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_io->attr.mode = 0600;
 	b->legacy_io->read = pci_read_legacy_io;
 	b->legacy_io->write = pci_write_legacy_io;
+	/* See pci_create_attr() for motivation */
+	b->legacy_io->llseek = pci_llseek_resource;
 	b->legacy_io->mmap = pci_mmap_legacy_io;
 	b->legacy_io->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_io);
@@ -981,6 +989,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_mem->size = 1024*1024;
 	b->legacy_mem->attr.mode = 0600;
 	b->legacy_mem->mmap = pci_mmap_legacy_mem;
+	/* See pci_create_attr() for motivation */
+	b->legacy_io->llseek = pci_llseek_resource;
 	b->legacy_mem->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_mem);
 	error = device_create_bin_file(&b->dev, b->legacy_mem);
@@ -1138,6 +1148,14 @@ static ssize_t pci_write_resource_io(struct file *filp, struct kobject *kobj,
 	return pci_resource_io(filp, kobj, attr, buf, off, count, true);
 }
 
+static loff_t pci_llseek_resource(struct file *filep,
+				  struct kobject *kobj __always_unused,
+				  struct bin_attribute *attr,
+				  loff_t offset, int whence)
+{
+	return fixed_size_llseek(filep, offset, whence, attr->size);
+}
+
 /**
  * pci_remove_resource_files - cleanup resource files
  * @pdev: dev to cleanup
@@ -1195,8 +1213,15 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
 			res_attr->mmap = pci_mmap_resource_uc;
 		}
 	}
-	if (res_attr->mmap)
+	if (res_attr->mmap) {
 		res_attr->f_mapping = iomem_get_mapping;
+		/*
+		 * generic_file_llseek() consults f_mapping->host to determine
+		 * the file size. As iomem_inode knows nothing about the
+		 * attribute, it's not going to work, so override it as well.
+		 */
+		res_attr->llseek = pci_llseek_resource;
+	}
 	res_attr->attr.name = res_attr_name;
 	res_attr->attr.mode = 0600;
 	res_attr->size = pci_resource_len(pdev, num);
-- 
2.34.1

Re: [PATCH v4 2/2] PCI: Implement custom llseek for sysfs resource entries

[kernel test robot]

Hi Valentine,

kernel test robot noticed the following build warnings:

[auto build test WARNING on f7757129e3dea336c407551c98f50057c22bb266]

url:    https://github.com/intel-lab-lkp/linux/commits/Valentine-Sinitsyn/PCI-Implement-custom-llseek-for-sysfs-resource-entries/20230822-205513
base:   f7757129e3dea336c407551c98f50057c22bb266
patch link:    https://lore.kernel.org/r/20230822115455.310222-2-valesini%40yandex-team.ru
patch subject: [PATCH v4 2/2] PCI: Implement custom llseek for sysfs resource entries
config: alpha-allnoconfig (https://download.01.org/0day-ci/archive/20230823/202308230126.O8xXYkdt-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230823/202308230126.O8xXYkdt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308230126.O8xXYkdt-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/pci/pci-sysfs.c:840:15: warning: 'pci_llseek_resource' used but never defined
     840 | static loff_t pci_llseek_resource(struct file *filep,
         |               ^~~~~~~~~~~~~~~~~~~


vim +/pci_llseek_resource +840 drivers/pci/pci-sysfs.c

   839	
 > 840	static loff_t pci_llseek_resource(struct file *filep,
   841					  struct kobject *kobj,
   842					  struct bin_attribute *attr,
   843					  loff_t offset, int whence);
   844	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v5 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

[Valentine Sinitsyn]

As of now, seeking in sysfs files is handled by generic_file_llseek().
There are situations where one may want to customize seeking logic:

- Many sysfs entries are fixed files while generic_file_llseek() accepts
  past-the-end positions. Not only being useless by itself, this
  also means a bug in userspace code will trigger not at lseek(), but at
  some later point making debugging harder.
- generic_file_llseek() relies on f_mapping->host to get the file size
  which might not be correct for all sysfs entries.
  See commit 636b21b50152 ("PCI: Revoke mappings like devmem") as an example.

Implement llseek method to override this behavior at sysfs attribute
level. The method is optional, and if it is absent,
generic_file_llseek() is called to preserve backwards compatibility.

Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
---
v5:
        - Fix builds without PCI mmap support (e.g. Alpha)
v4:
        - Fix builds which #define HAVE_PCI_LEGACY (e.g. PowerPC)
v3:
        - Grammar fixes
        - Add base-patch: and prerequisite-patch-id: to make kernel test
          robot happy
v2:
        - Add infrastructure to customize llseek per sysfs entry type
        - Override llseek for PCI sysfs entries with fixed_file_llseek()
 fs/kernfs/file.c       | 14 +++++++++++++-
 fs/sysfs/file.c        | 13 +++++++++++++
 include/linux/kernfs.h |  1 +
 include/linux/sysfs.h  |  2 ++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 180906c36f51..6166bf74d5b8 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -903,6 +903,18 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait)
 	return ret;
 }
 
+static loff_t kernfs_fop_llseek(struct file *file, loff_t offset, int whence)
+{
+	struct kernfs_open_file *of = kernfs_of(file);
+	const struct kernfs_ops *ops;
+
+	ops = kernfs_ops(of->kn);
+	if (ops->llseek)
+		return ops->llseek(of, offset, whence);
+	else
+		return generic_file_llseek(file, offset, whence);
+}
+
 static void kernfs_notify_workfn(struct work_struct *work)
 {
 	struct kernfs_node *kn;
@@ -1005,7 +1017,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify);
 const struct file_operations kernfs_file_fops = {
 	.read_iter	= kernfs_fop_read_iter,
 	.write_iter	= kernfs_fop_write_iter,
-	.llseek		= generic_file_llseek,
+	.llseek		= kernfs_fop_llseek,
 	.mmap		= kernfs_fop_mmap,
 	.open		= kernfs_fop_open,
 	.release	= kernfs_fop_release,
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index a12ac0356c69..6b7652fb8050 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -167,6 +167,18 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of,
 	return battr->mmap(of->file, kobj, battr, vma);
 }
 
+static loff_t sysfs_kf_bin_llseek(struct kernfs_open_file *of, loff_t offset,
+				  int whence)
+{
+	struct bin_attribute *battr = of->kn->priv;
+	struct kobject *kobj = of->kn->parent->priv;
+
+	if (battr->llseek)
+		return battr->llseek(of->file, kobj, battr, offset, whence);
+	else
+		return generic_file_llseek(of->file, offset, whence);
+}
+
 static int sysfs_kf_bin_open(struct kernfs_open_file *of)
 {
 	struct bin_attribute *battr = of->kn->priv;
@@ -249,6 +261,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = {
 	.write		= sysfs_kf_bin_write,
 	.mmap		= sysfs_kf_bin_mmap,
 	.open		= sysfs_kf_bin_open,
+	.llseek		= sysfs_kf_bin_llseek,
 };
 
 int sysfs_add_file_mode_ns(struct kernfs_node *parent,
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 73f5c120def8..9ed535930259 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -316,6 +316,7 @@ struct kernfs_ops {
 			 struct poll_table_struct *pt);
 
 	int (*mmap)(struct kernfs_open_file *of, struct vm_area_struct *vma);
+	loff_t (*llseek)(struct kernfs_open_file *of, loff_t offset, int whence);
 };
 
 /*
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index fd3fe5c8c17f..b717a70219f6 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -181,6 +181,8 @@ struct bin_attribute {
 			char *, loff_t, size_t);
 	ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
 			 char *, loff_t, size_t);
+	loff_t (*llseek)(struct file *, struct kobject *, struct bin_attribute *,
+			 loff_t, int);
 	int (*mmap)(struct file *, struct kobject *, struct bin_attribute *attr,
 		    struct vm_area_struct *vma);
 };

base-commit: 89bf6209cad66214d3774dac86b6bbf2aec6a30d
-- 
2.34.1

[PATCH v5 2/2] PCI: Implement custom llseek for sysfs resource entries

[Valentine Sinitsyn]

Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"), mmappable
sysfs entries have started to receive their f_mapping from the iomem
pseudo filesystem, so that CONFIG_IO_STRICT_DEVMEM is honored in sysfs
(and procfs) as well as in /dev/[k]mem.

This resulted in a userspace-visible regression:

1. Open a sysfs PCI resource file (eg. /sys/bus/pci/devices/*/resource0)
2. Use lseek(fd, 0, SEEK_END) to determine its size

Expected result: a PCI region size is returned.
Actual result: 0 is returned.

The reason is that PCI resource files residing in sysfs use
generic_file_llseek(), which relies on f_mapping->host inode to get the
file size. As f_mapping is now redefined, f_mapping->host points to an
anonymous zero-sized iomem_inode which has nothing to do with sysfs file
in question.

Implement a custom llseek method for sysfs PCI resources, which is
almost the same as proc_bus_pci_lseek() used for procfs entries.

This makes sysfs and procfs entries consistent with regards to seeking,
but also introduces userspace-visible changes to seeking PCI resources
in sysfs:

- SEEK_DATA and SEEK_HOLE are no longer supported;
- Seeking past the end of the file is prohibited while previously
  offsets up to MAX_NON_LFS were accepted (reading from these offsets
  was always invalid).

Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci-sysfs.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index ab32a91f287b..03ac21eeb9bb 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -835,6 +835,14 @@ static const struct attribute_group pci_dev_config_attr_group = {
 	.is_bin_visible = pci_dev_config_attr_is_visible,
 };
 
+static loff_t pci_llseek_resource(struct file *filep,
+				  struct kobject *kobj __always_unused,
+				  struct bin_attribute *attr,
+				  loff_t offset, int whence)
+{
+	return fixed_size_llseek(filep, offset, whence, attr->size);
+}
+
 #ifdef HAVE_PCI_LEGACY
 /**
  * pci_read_legacy_io - read byte(s) from legacy I/O port space
@@ -967,6 +975,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_io->attr.mode = 0600;
 	b->legacy_io->read = pci_read_legacy_io;
 	b->legacy_io->write = pci_write_legacy_io;
+	/* See pci_create_attr() for motivation */
+	b->legacy_io->llseek = pci_llseek_resource;
 	b->legacy_io->mmap = pci_mmap_legacy_io;
 	b->legacy_io->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_io);
@@ -981,6 +991,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_mem->size = 1024*1024;
 	b->legacy_mem->attr.mode = 0600;
 	b->legacy_mem->mmap = pci_mmap_legacy_mem;
+	/* See pci_create_attr() for motivation */
+	b->legacy_io->llseek = pci_llseek_resource;
 	b->legacy_mem->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_mem);
 	error = device_create_bin_file(&b->dev, b->legacy_mem);
@@ -1195,8 +1207,15 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
 			res_attr->mmap = pci_mmap_resource_uc;
 		}
 	}
-	if (res_attr->mmap)
+	if (res_attr->mmap) {
 		res_attr->f_mapping = iomem_get_mapping;
+		/*
+		 * generic_file_llseek() consults f_mapping->host to determine
+		 * the file size. As iomem_inode knows nothing about the
+		 * attribute, it's not going to work, so override it as well.
+		 */
+		res_attr->llseek = pci_llseek_resource;
+	}
 	res_attr->attr.name = res_attr_name;
 	res_attr->attr.mode = 0600;
 	res_attr->size = pci_resource_len(pdev, num);
-- 
2.34.1

Re: [PATCH v5 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

[Greg Kroah-Hartman]

On Wed, Aug 23, 2023 at 05:58:33PM +0500, Valentine Sinitsyn wrote:
> As of now, seeking in sysfs files is handled by generic_file_llseek().
> There are situations where one may want to customize seeking logic:
> 
> - Many sysfs entries are fixed files while generic_file_llseek() accepts
>   past-the-end positions. Not only being useless by itself, this
>   also means a bug in userspace code will trigger not at lseek(), but at
>   some later point making debugging harder.
> - generic_file_llseek() relies on f_mapping->host to get the file size
>   which might not be correct for all sysfs entries.
>   See commit 636b21b50152 ("PCI: Revoke mappings like devmem") as an example.
> 
> Implement llseek method to override this behavior at sysfs attribute
> level. The method is optional, and if it is absent,
> generic_file_llseek() is called to preserve backwards compatibility.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
> ---
> v5:
>         - Fix builds without PCI mmap support (e.g. Alpha)
> v4:
>         - Fix builds which #define HAVE_PCI_LEGACY (e.g. PowerPC)
> v3:
>         - Grammar fixes
>         - Add base-patch: and prerequisite-patch-id: to make kernel test
>           robot happy
> v2:
>         - Add infrastructure to customize llseek per sysfs entry type
>         - Override llseek for PCI sysfs entries with fixed_file_llseek()
>  fs/kernfs/file.c       | 14 +++++++++++++-
>  fs/sysfs/file.c        | 13 +++++++++++++
>  include/linux/kernfs.h |  1 +
>  include/linux/sysfs.h  |  2 ++
>  4 files changed, 29 insertions(+), 1 deletion(-)
> 

Due to the high rate of errors and respins of this, I'm going to wait
until after 5.7-rc1 is out before adding it to my tree so that things
can calm down a bit...

thanks,

greg k-h

Re: [PATCH v5 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

[Greg Kroah-Hartman]

On Wed, Aug 23, 2023 at 04:37:49PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Aug 23, 2023 at 05:58:33PM +0500, Valentine Sinitsyn wrote:
> > As of now, seeking in sysfs files is handled by generic_file_llseek().
> > There are situations where one may want to customize seeking logic:
> > 
> > - Many sysfs entries are fixed files while generic_file_llseek() accepts
> >   past-the-end positions. Not only being useless by itself, this
> >   also means a bug in userspace code will trigger not at lseek(), but at
> >   some later point making debugging harder.
> > - generic_file_llseek() relies on f_mapping->host to get the file size
> >   which might not be correct for all sysfs entries.
> >   See commit 636b21b50152 ("PCI: Revoke mappings like devmem") as an example.
> > 
> > Implement llseek method to override this behavior at sysfs attribute
> > level. The method is optional, and if it is absent,
> > generic_file_llseek() is called to preserve backwards compatibility.
> > 
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
> > ---
> > v5:
> >         - Fix builds without PCI mmap support (e.g. Alpha)
> > v4:
> >         - Fix builds which #define HAVE_PCI_LEGACY (e.g. PowerPC)
> > v3:
> >         - Grammar fixes
> >         - Add base-patch: and prerequisite-patch-id: to make kernel test
> >           robot happy
> > v2:
> >         - Add infrastructure to customize llseek per sysfs entry type
> >         - Override llseek for PCI sysfs entries with fixed_file_llseek()
> >  fs/kernfs/file.c       | 14 +++++++++++++-
> >  fs/sysfs/file.c        | 13 +++++++++++++
> >  include/linux/kernfs.h |  1 +
> >  include/linux/sysfs.h  |  2 ++
> >  4 files changed, 29 insertions(+), 1 deletion(-)
> > 
> 
> Due to the high rate of errors and respins of this, I'm going to wait
> until after 5.7-rc1 is out before adding it to my tree so that things
> can calm down a bit...

{sigh}

I don't know what kernel version we are at anymore, it should be
"6.6-rc1".

thanks,

greg k-h

Re: [PATCH v5 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

[Valentin Sinitsyn]

[...]

>> Due to the high rate of errors and respins of this, I'm going to wait
>> until after 5.7-rc1 is out before adding it to my tree so that things
>> can calm down a bit...
Ack. Sorry for the fuss, I believe v5 should be final, unless someone 
steps in with essential review comments.

Thanks,
Valentine

> 
> {sigh}
> 
> I don't know what kernel version we are at anymore, it should be
> "6.6-rc1".
> 
> thanks,
> 
> greg k-h

Re: [PATCH v5 2/2] PCI: Implement custom llseek for sysfs resource entries

[kernel test robot]

Hi Valentine,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 89bf6209cad66214d3774dac86b6bbf2aec6a30d]

url:    https://github.com/intel-lab-lkp/linux/commits/Valentine-Sinitsyn/PCI-Implement-custom-llseek-for-sysfs-resource-entries/20230823-210101
base:   89bf6209cad66214d3774dac86b6bbf2aec6a30d
patch link:    https://lore.kernel.org/r/20230823125834.492298-2-valesini%40yandex-team.ru
patch subject: [PATCH v5 2/2] PCI: Implement custom llseek for sysfs resource entries
config: arc-randconfig-r023-20230824 (https://download.01.org/0day-ci/archive/20230825/202308252133.YO5zs5rv-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230825/202308252133.YO5zs5rv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308252133.YO5zs5rv-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/pci/pci-sysfs.c:1268:12: warning: no previous prototype for 'pci_create_resource_files' [-Wmissing-prototypes]
    1268 | int __weak pci_create_resource_files(struct pci_dev *dev) { return 0; }
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/pci/pci-sysfs.c:1269:13: warning: no previous prototype for 'pci_remove_resource_files' [-Wmissing-prototypes]
    1269 | void __weak pci_remove_resource_files(struct pci_dev *dev) { return; }
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/pci-sysfs.c:838:15: warning: 'pci_llseek_resource' defined but not used [-Wunused-function]
     838 | static loff_t pci_llseek_resource(struct file *filep,
         |               ^~~~~~~~~~~~~~~~~~~


vim +/pci_llseek_resource +838 drivers/pci/pci-sysfs.c

   837	
 > 838	static loff_t pci_llseek_resource(struct file *filep,
   839					  struct kobject *kobj __always_unused,
   840					  struct bin_attribute *attr,
   841					  loff_t offset, int whence)
   842	{
   843		return fixed_size_llseek(filep, offset, whence, attr->size);
   844	}
   845	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v5 2/2] PCI: Implement custom llseek for sysfs resource entries

[Valentin Sinitsyn]

Hi all,

On 25.08.2023 19:10, kernel test robot wrote:
> Hi Valentine,
> 
> kernel test robot noticed the following build warnings:
> 
[..]
> All warnings (new ones prefixed by >>):
> 
>     drivers/pci/pci-sysfs.c:1268:12: warning: no previous prototype for 'pci_create_resource_files' [-Wmissing-prototypes]
>      1268 | int __weak pci_create_resource_files(struct pci_dev *dev) { return 0; }
>           |            ^~~~~~~~~~~~~~~~~~~~~~~~~
>     drivers/pci/pci-sysfs.c:1269:13: warning: no previous prototype for 'pci_remove_resource_files' [-Wmissing-prototypes]
>      1269 | void __weak pci_remove_resource_files(struct pci_dev *dev) { return; }
>           |             ^~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/pci/pci-sysfs.c:838:15: warning: 'pci_llseek_resource' defined but not used [-Wunused-function]
>       838 | static loff_t pci_llseek_resource(struct file *filep,
>           |               ^~~~~~~~~~~~~~~~~~~
I'm not sure if I should do anything about it: arc doesn't HAVE_* 
anything PCI-related, and pci-sysfs doesn't seem to be written with this 
case in mind. Guarding pci_llseek_resource() with an #ifdef 
HAVE_AT_LEAST_SOMETHING is trivial, but it feels more like a patch to 
silence a bot than a proper fix.

Any thoughts / objections on this?

Best,
Valentine

Re: [PATCH v5 2/2] PCI: Implement custom llseek for sysfs resource entries

[Bjorn Helgaas]

On Mon, Aug 28, 2023 at 12:22:54PM +0500, Valentin Sinitsyn wrote:
> On 25.08.2023 19:10, kernel test robot wrote:
> > Hi Valentine,
> > 
> > kernel test robot noticed the following build warnings:
> > 
> [..]
> > All warnings (new ones prefixed by >>):
> > 
> >     drivers/pci/pci-sysfs.c:1268:12: warning: no previous prototype for 'pci_create_resource_files' [-Wmissing-prototypes]
> >      1268 | int __weak pci_create_resource_files(struct pci_dev *dev) { return 0; }
> >           |            ^~~~~~~~~~~~~~~~~~~~~~~~~
> >     drivers/pci/pci-sysfs.c:1269:13: warning: no previous prototype for 'pci_remove_resource_files' [-Wmissing-prototypes]
> >      1269 | void __weak pci_remove_resource_files(struct pci_dev *dev) { return; }
> >           |             ^~~~~~~~~~~~~~~~~~~~~~~~~
> > > > drivers/pci/pci-sysfs.c:838:15: warning: 'pci_llseek_resource' defined but not used [-Wunused-function]
> >       838 | static loff_t pci_llseek_resource(struct file *filep,
> >           |               ^~~~~~~~~~~~~~~~~~~
> I'm not sure if I should do anything about it: arc doesn't HAVE_* anything
> PCI-related, and pci-sysfs doesn't seem to be written with this case in
> mind. Guarding pci_llseek_resource() with an #ifdef HAVE_AT_LEAST_SOMETHING
> is trivial, but it feels more like a patch to silence a bot than a proper
> fix.

Looks like it will require some untangling to figure out a nice fix,
but I think you *do* need to do something about it because I don't
want to add build errors on any arch.

Bjorn

Re: [PATCH v5 2/2] PCI: Implement custom llseek for sysfs resource entries

[Bjorn Helgaas]

On Tue, Aug 29, 2023 at 03:02:29PM -0500, Bjorn Helgaas wrote:
> On Mon, Aug 28, 2023 at 12:22:54PM +0500, Valentin Sinitsyn wrote:
> > On 25.08.2023 19:10, kernel test robot wrote:
> > > Hi Valentine,
> > > 
> > > kernel test robot noticed the following build warnings:
> > > 
> > [..]
> > > All warnings (new ones prefixed by >>):
> > > 
> > >     drivers/pci/pci-sysfs.c:1268:12: warning: no previous prototype for 'pci_create_resource_files' [-Wmissing-prototypes]
> > >      1268 | int __weak pci_create_resource_files(struct pci_dev *dev) { return 0; }
> > >           |            ^~~~~~~~~~~~~~~~~~~~~~~~~
> > >     drivers/pci/pci-sysfs.c:1269:13: warning: no previous prototype for 'pci_remove_resource_files' [-Wmissing-prototypes]
> > >      1269 | void __weak pci_remove_resource_files(struct pci_dev *dev) { return; }
> > >           |             ^~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > drivers/pci/pci-sysfs.c:838:15: warning: 'pci_llseek_resource' defined but not used [-Wunused-function]
> > >       838 | static loff_t pci_llseek_resource(struct file *filep,
> > >           |               ^~~~~~~~~~~~~~~~~~~
> > I'm not sure if I should do anything about it: arc doesn't HAVE_* anything
> > PCI-related, and pci-sysfs doesn't seem to be written with this case in
> > mind. Guarding pci_llseek_resource() with an #ifdef HAVE_AT_LEAST_SOMETHING
> > is trivial, but it feels more like a patch to silence a bot than a proper
> > fix.
> 
> Looks like it will require some untangling to figure out a nice fix,
> but I think you *do* need to do something about it because I don't
> want to add build errors on any arch.

P.S. This patch:
https://lore.kernel.org/r/20230810141947.1236730-5-arnd@kernel.org is
queued up for v6.6-rc1 and should help with the first two warnings.

Bjorn

[PATCH v6 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

[Valentine Sinitsyn]

As of now, seeking in sysfs files is handled by generic_file_llseek().
There are situations where one may want to customize seeking logic:

- Many sysfs entries are fixed files while generic_file_llseek() accepts
  past-the-end positions. Not only being useless by itself, this
  also means a bug in userspace code will trigger not at lseek(), but at
  some later point making debugging harder.
- generic_file_llseek() relies on f_mapping->host to get the file size
  which might not be correct for all sysfs entries.
  See commit 636b21b50152 ("PCI: Revoke mappings like devmem") as an example.

Implement llseek method to override this behavior at sysfs attribute
level. The method is optional, and if it is absent,
generic_file_llseek() is called to preserve backwards compatibility.

Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
---
Looks like a mere __maybe_unused is the cleanest way to fix the issue.

v6:
        - Mark pci_llseek_resource() as __maybe_unused
        - Fix a typo in pci_create_legacy_files()
v5:
        - Fix builds without PCI mmap support (e.g. Alpha)
v4:
        - Fix builds which #define HAVE_PCI_LEGACY (e.g. PowerPC)
v3:
        - Grammar fixes
        - Add base-patch: and prerequisite-patch-id: to make kernel test
          robot happy
v2:
        - Add infrastructure to customize llseek per sysfs entry type
        - Override llseek for PCI sysfs entries with fixed_file_llseek()

 fs/kernfs/file.c       | 14 +++++++++++++-
 fs/sysfs/file.c        | 13 +++++++++++++
 include/linux/kernfs.h |  1 +
 include/linux/sysfs.h  |  2 ++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 180906c36f51..6166bf74d5b8 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -903,6 +903,18 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait)
 	return ret;
 }
 
+static loff_t kernfs_fop_llseek(struct file *file, loff_t offset, int whence)
+{
+	struct kernfs_open_file *of = kernfs_of(file);
+	const struct kernfs_ops *ops;
+
+	ops = kernfs_ops(of->kn);
+	if (ops->llseek)
+		return ops->llseek(of, offset, whence);
+	else
+		return generic_file_llseek(file, offset, whence);
+}
+
 static void kernfs_notify_workfn(struct work_struct *work)
 {
 	struct kernfs_node *kn;
@@ -1005,7 +1017,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify);
 const struct file_operations kernfs_file_fops = {
 	.read_iter	= kernfs_fop_read_iter,
 	.write_iter	= kernfs_fop_write_iter,
-	.llseek		= generic_file_llseek,
+	.llseek		= kernfs_fop_llseek,
 	.mmap		= kernfs_fop_mmap,
 	.open		= kernfs_fop_open,
 	.release	= kernfs_fop_release,
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index a12ac0356c69..6b7652fb8050 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -167,6 +167,18 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of,
 	return battr->mmap(of->file, kobj, battr, vma);
 }
 
+static loff_t sysfs_kf_bin_llseek(struct kernfs_open_file *of, loff_t offset,
+				  int whence)
+{
+	struct bin_attribute *battr = of->kn->priv;
+	struct kobject *kobj = of->kn->parent->priv;
+
+	if (battr->llseek)
+		return battr->llseek(of->file, kobj, battr, offset, whence);
+	else
+		return generic_file_llseek(of->file, offset, whence);
+}
+
 static int sysfs_kf_bin_open(struct kernfs_open_file *of)
 {
 	struct bin_attribute *battr = of->kn->priv;
@@ -249,6 +261,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = {
 	.write		= sysfs_kf_bin_write,
 	.mmap		= sysfs_kf_bin_mmap,
 	.open		= sysfs_kf_bin_open,
+	.llseek		= sysfs_kf_bin_llseek,
 };
 
 int sysfs_add_file_mode_ns(struct kernfs_node *parent,
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 2a36f3218b51..99aaa050ccb7 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -316,6 +316,7 @@ struct kernfs_ops {
 			 struct poll_table_struct *pt);
 
 	int (*mmap)(struct kernfs_open_file *of, struct vm_area_struct *vma);
+	loff_t (*llseek)(struct kernfs_open_file *of, loff_t offset, int whence);
 };
 
 /*
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index fd3fe5c8c17f..b717a70219f6 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -181,6 +181,8 @@ struct bin_attribute {
 			char *, loff_t, size_t);
 	ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
 			 char *, loff_t, size_t);
+	loff_t (*llseek)(struct file *, struct kobject *, struct bin_attribute *,
+			 loff_t, int);
 	int (*mmap)(struct file *, struct kobject *, struct bin_attribute *attr,
 		    struct vm_area_struct *vma);
 };

base-commit: 76be05d4fd6c91a3885298f1dc3efeef32846399
-- 
2.34.1

[PATCH v6 2/2] PCI: Implement custom llseek for sysfs resource entries

[Valentine Sinitsyn]

Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"), mmappable
sysfs entries have started to receive their f_mapping from the iomem
pseudo filesystem, so that CONFIG_IO_STRICT_DEVMEM is honored in sysfs
(and procfs) as well as in /dev/[k]mem.

This resulted in a userspace-visible regression:

1. Open a sysfs PCI resource file (eg. /sys/bus/pci/devices/*/resource0)
2. Use lseek(fd, 0, SEEK_END) to determine its size

Expected result: a PCI region size is returned.
Actual result: 0 is returned.

The reason is that PCI resource files residing in sysfs use
generic_file_llseek(), which relies on f_mapping->host inode to get the
file size. As f_mapping is now redefined, f_mapping->host points to an
anonymous zero-sized iomem_inode which has nothing to do with sysfs file
in question.

Implement a custom llseek method for sysfs PCI resources, which is
almost the same as proc_bus_pci_lseek() used for procfs entries.

This makes sysfs and procfs entries consistent with regards to seeking,
but also introduces userspace-visible changes to seeking PCI resources
in sysfs:

- SEEK_DATA and SEEK_HOLE are no longer supported;
- Seeking past the end of the file is prohibited while previously
  offsets up to MAX_NON_LFS were accepted (reading from these offsets
  was always invalid).

Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci-sysfs.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index d9eede2dbc0e..97c9c62d5e3e 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -835,6 +835,19 @@ static const struct attribute_group pci_dev_config_attr_group = {
 	.is_bin_visible = pci_dev_config_attr_is_visible,
 };
 
+/*
+ * llseek operation for mmappable PCI resources.
+ * May be left unused if the arch doesn't provide them.
+ */
+static __maybe_unused loff_t
+pci_llseek_resource(struct file *filep,
+		    struct kobject *kobj __always_unused,
+		    struct bin_attribute *attr,
+		    loff_t offset, int whence)
+{
+	return fixed_size_llseek(filep, offset, whence, attr->size);
+}
+
 #ifdef HAVE_PCI_LEGACY
 /**
  * pci_read_legacy_io - read byte(s) from legacy I/O port space
@@ -967,6 +980,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_io->attr.mode = 0600;
 	b->legacy_io->read = pci_read_legacy_io;
 	b->legacy_io->write = pci_write_legacy_io;
+	/* See pci_create_attr() for motivation */
+	b->legacy_io->llseek = pci_llseek_resource;
 	b->legacy_io->mmap = pci_mmap_legacy_io;
 	b->legacy_io->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_io);
@@ -981,6 +996,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_mem->size = 1024*1024;
 	b->legacy_mem->attr.mode = 0600;
 	b->legacy_mem->mmap = pci_mmap_legacy_mem;
+	/* See pci_create_attr() for motivation */
+	b->legacy_mem->llseek = pci_llseek_resource;
 	b->legacy_mem->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_mem);
 	error = device_create_bin_file(&b->dev, b->legacy_mem);
@@ -1199,8 +1216,15 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
 			res_attr->mmap = pci_mmap_resource_uc;
 		}
 	}
-	if (res_attr->mmap)
+	if (res_attr->mmap) {
 		res_attr->f_mapping = iomem_get_mapping;
+		/*
+		 * generic_file_llseek() consults f_mapping->host to determine
+		 * the file size. As iomem_inode knows nothing about the
+		 * attribute, it's not going to work, so override it as well.
+		 */
+		res_attr->llseek = pci_llseek_resource;
+	}
 	res_attr->attr.name = res_attr_name;
 	res_attr->attr.mode = 0600;
 	res_attr->size = pci_resource_len(pdev, num);
-- 
2.34.1

Re: [PATCH v6 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

[kernel test robot]

Hello,

kernel test robot noticed "WARNING:at_fs/kernfs/file.c:#kernfs_ops" on:

commit: c6eefad61209dab3de7446cf8151d38e70440484 ("[PATCH v6 1/2] kernfs: sysfs: support custom llseek method for sysfs entries")
url: https://github.com/intel-lab-lkp/linux/commits/Valentine-Sinitsyn/PCI-Implement-custom-llseek-for-sysfs-resource-entries/20230902-235234
patch link: https://lore.kernel.org/all/20230902155038.1661970-1-valesini@yandex-team.ru/
patch subject: [PATCH v6 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

in testcase: boot

compiler: clang-16
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)



If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202309051442.bd6f9879-oliver.sang@intel.com


[   96.320907][  T252] ------------[ cut here ]------------
[ 96.321715][ T252] WARNING: CPU: 1 PID: 252 at fs/kernfs/file.c:109 kernfs_ops (fs/kernfs/file.c:109) 
[   96.322853][  T252] Modules linked in:
[   96.323438][  T252] CPU: 1 PID: 252 Comm: systemd-logind Not tainted 6.5.0-10888-gc6eefad61209 #1
[   96.324966][  T252] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 96.326330][ T252] RIP: 0010:kernfs_ops (fs/kernfs/file.c:109) 
[ 96.327026][ T252] Code: 48 83 c3 68 48 89 d8 48 c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 de f3 e2 ff 48 8b 03 5b 41 5e 41 5f 31 c9 31 ff 31 f6 c3 cc <0f> 0b eb d2 44 89 f1 80 e1 07 fe c1 38 c1 7c 91 4c 89 f7 e8 76 f3
All code
========
   0:	48 83 c3 68          	add    $0x68,%rbx
   4:	48 89 d8             	mov    %rbx,%rax
   7:	48 c1 e8 03          	shr    $0x3,%rax
   b:	42 80 3c 38 00       	cmpb   $0x0,(%rax,%r15,1)
  10:	74 08                	je     0x1a
  12:	48 89 df             	mov    %rbx,%rdi
  15:	e8 de f3 e2 ff       	call   0xffffffffffe2f3f8
  1a:	48 8b 03             	mov    (%rbx),%rax
  1d:	5b                   	pop    %rbx
  1e:	41 5e                	pop    %r14
  20:	41 5f                	pop    %r15
  22:	31 c9                	xor    %ecx,%ecx
  24:	31 ff                	xor    %edi,%edi
  26:	31 f6                	xor    %esi,%esi
  28:	c3                   	ret
  29:	cc                   	int3
  2a:*	0f 0b                	ud2		<-- trapping instruction
  2c:	eb d2                	jmp    0x0
  2e:	44 89 f1             	mov    %r14d,%ecx
  31:	80 e1 07             	and    $0x7,%cl
  34:	fe c1                	inc    %cl
  36:	38 c1                	cmp    %al,%cl
  38:	7c 91                	jl     0xffffffffffffffcb
  3a:	4c 89 f7             	mov    %r14,%rdi
  3d:	e8                   	.byte 0xe8
  3e:	76 f3                	jbe    0x33

Code starting with the faulting instruction
===========================================
   0:	0f 0b                	ud2
   2:	eb d2                	jmp    0xffffffffffffffd6
   4:	44 89 f1             	mov    %r14d,%ecx
   7:	80 e1 07             	and    $0x7,%cl
   a:	fe c1                	inc    %cl
   c:	38 c1                	cmp    %al,%cl
   e:	7c 91                	jl     0xffffffffffffffa1
  10:	4c 89 f7             	mov    %r14,%rdi
  13:	e8                   	.byte 0xe8
  14:	76 f3                	jbe    0x9
[   96.329439][  T252] RSP: 0018:ffff8881638e7ea0 EFLAGS: 00010246
[   96.330261][  T252] RAX: 0000000000000000 RBX: ffff888111e4a2b8 RCX: 0000000000000000
[   96.331346][  T252] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[   96.332425][  T252] RBP: dffffc0000000000 R08: 0000000000000000 R09: 0000000000000000
[   96.333503][  T252] R10: 0000000000000000 R11: ffffffff8184fd65 R12: ffff8881538a5c00
[   96.334649][  T252] R13: ffff888161bc5ec0 R14: ffff888111e4a350 R15: dffffc0000000000
[   96.335717][  T252] FS:  00007f2d9c6a8980(0000) GS:ffff8883aeb00000(0000) knlGS:0000000000000000
[   96.336833][  T252] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   96.337593][  T252] CR2: 00005600c8ef1595 CR3: 0000000162f96000 CR4: 00000000000406a0
[   96.338544][  T252] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   96.339487][  T252] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   96.340472][  T252] Call Trace:
[   96.340983][  T252]  <TASK>
[ 96.341447][ T252] ? __warn (kernel/panic.c:673) 
[ 96.342078][ T252] ? kernfs_ops (fs/kernfs/file.c:109) 
[ 96.342740][ T252] ? kernfs_ops (fs/kernfs/file.c:109) 
[ 96.343391][ T252] ? report_bug (lib/bug.c:?) 
[ 96.344046][ T252] ? find_held_lock (kernel/locking/lockdep.c:?) 
[ 96.344738][ T252] ? handle_bug (arch/x86/kernel/traps.c:237) 
[ 96.345314][ T252] ? exc_invalid_op (arch/x86/kernel/traps.c:258) 
[ 96.345963][ T252] ? asm_exc_invalid_op (arch/x86/include/asm/idtentry.h:568) 
[ 96.346694][ T252] ? __cfi_kernfs_fop_llseek (fs/kernfs/file.c:907) 
[ 96.347473][ T252] ? kernfs_ops (fs/kernfs/file.c:109) 
[ 96.348135][ T252] kernfs_fop_llseek (fs/kernfs/file.c:911) 
[ 96.348869][ T252] ksys_lseek (fs/read_write.c:289) 
[ 96.349497][ T252] do_syscall_64 (arch/x86/entry/common.c:?) 
[ 96.350141][ T252] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120) 
[   96.350952][  T252] RIP: 0033:0x7f2d9cf82fc7
[ 96.351602][ T252] Code: c7 c0 ff ff ff ff c3 0f 1f 40 00 48 8b 15 c1 ee 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb ba 0f 1f 00 b8 08 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 01 c3 48 8b 15 99 ee 0c 00 f7 d8 64 89 02 48
All code
========
   0:	c7 c0 ff ff ff ff    	mov    $0xffffffff,%eax
   6:	c3                   	ret
   7:	0f 1f 40 00          	nopl   0x0(%rax)
   b:	48 8b 15 c1 ee 0c 00 	mov    0xceec1(%rip),%rdx        # 0xceed3
  12:	f7 d8                	neg    %eax
  14:	64 89 02             	mov    %eax,%fs:(%rdx)
  17:	48 c7 c0 ff ff ff ff 	mov    $0xffffffffffffffff,%rax
  1e:	eb ba                	jmp    0xffffffffffffffda
  20:	0f 1f 00             	nopl   (%rax)
  23:	b8 08 00 00 00       	mov    $0x8,%eax
  28:	0f 05                	syscall
  2a:*	48 3d 00 f0 ff ff    	cmp    $0xfffffffffffff000,%rax		<-- trapping instruction
  30:	77 01                	ja     0x33
  32:	c3                   	ret
  33:	48 8b 15 99 ee 0c 00 	mov    0xcee99(%rip),%rdx        # 0xceed3
  3a:	f7 d8                	neg    %eax
  3c:	64 89 02             	mov    %eax,%fs:(%rdx)
  3f:	48                   	rex.W

Code starting with the faulting instruction
===========================================
   0:	48 3d 00 f0 ff ff    	cmp    $0xfffffffffffff000,%rax
   6:	77 01                	ja     0x9
   8:	c3                   	ret
   9:	48 8b 15 99 ee 0c 00 	mov    0xcee99(%rip),%rdx        # 0xceea9
  10:	f7 d8                	neg    %eax
  12:	64 89 02             	mov    %eax,%fs:(%rdx)
  15:	48                   	rex.W
[   96.357571][  T252] RSP: 002b:00007fff4e4c3fc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000008
[   96.358740][  T252] RAX: ffffffffffffffda RBX: 0000563f1ffa0a80 RCX: 00007f2d9cf82fc7
[   96.359842][  T252] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000007
[   96.360873][  T252] RBP: 0000563f1ffa0a80 R08: 00007fff4e4c3fc0 R09: 0000000000000000
[   96.361934][  T252] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[   96.363024][  T252] R13: 0000000000000000 R14: 00007fff4e4c4188 R15: 00007fff4e4c41a0
[   96.364155][  T252]  </TASK>
[   96.364719][  T252] irq event stamp: 64209
[ 96.365341][ T252] hardirqs last enabled at (64219): __up_console_sem (arch/x86/include/asm/irqflags.h:26 arch/x86/include/asm/irqflags.h:67 arch/x86/include/asm/irqflags.h:127 kernel/printk/printk.c:347) 
[ 96.366558][ T252] hardirqs last disabled at (64228): __up_console_sem (kernel/printk/printk.c:345) 
[ 96.367774][ T252] softirqs last enabled at (64128): __do_softirq (arch/x86/include/asm/preempt.h:27 kernel/softirq.c:400 kernel/softirq.c:582) 
[ 96.368971][ T252] softirqs last disabled at (64119): __irq_exit_rcu (kernel/softirq.c:612) 
[   96.370158][  T252] ---[ end trace 0000000000000000 ]---
Starting LKP bootstrap...
Starting /etc/rc.local Compatibility...
Starting OpenBSD Secure Shell server...
Starting Permit User Sessions...
[FAILED] Failed to start LSB: OpenIPMI Driver init script.
See 'systemctl status openipmi.service' for details.
[  OK  ] Started User Login Management.
[  OK  ] Started LKP bootstrap.
[  OK  ] Started LSB: Load kernel image with kexec.
LKP: ttyS0: 296: skip deploy intel ucode as no ucode is specified
[  OK  ] Finished Permit User Sessions.
[  OK  ] Started OpenBSD Secure Shell server.
LKP: ttyS0: 296: Kernel tests: Boot OK!
LKP: ttyS0: 296: HOSTNAME vm-snb, MAC 52:54:00:12:34:56, kernel 6.5.0-10888-gc6eefad61209 1
LKP: ttyS0: 296:  /lkp/lkp/src/bin/run-lkp /lkp/jobs/scheduled/vm-meta-300/boot-1-debian-11.1-x86_64-20220510.cgz-x86_64-randconfig-071-20230903-c6eefad61209-20230903-11038-o17p1l-0.yaml
[  104.602900][  T165] kmemleak: Automatic memory scanning thread ended
[  OK  ] Started System Logging Service.
[  106.970306][  T310] is_virt=true
[  106.970347][  T310]
[  108.079565][  T310] lkp: kernel tainted state: 512
[  108.079600][  T310]
[  108.704632][  T310] LKP: stdout: 296: Kernel tests: Boot OK!
[  108.704672][  T310]
[  112.730026][  T310] LKP: stdout: 296: HOSTNAME vm-snb, MAC 52:54:00:12:34:56, kernel 6.5.0-10888-gc6eefad61209 1
[  112.730070][  T310]
[  112.732401][  T310] NO_NETWORK=
[  112.732437][  T310]
[  118.521298][  T310] LKP: stdout: 296:  /lkp/lkp/src/bin/run-lkp /lkp/jobs/scheduled/vm-meta-300/boot-1-debian-11.1-x86_64-20220510.cgz-x86_64-randconfig-071-20230903-c6eefad61209-20230903-11038-o17p1l-0.yaml
[  118.521368][  T310]
[  118.537313][  T310] RESULT_ROOT=/result/boot/1/vm-snb/debian-11.1-x86_64-20220510.cgz/x86_64-randconfig-071-20230903/clang-16/c6eefad61209dab3de7446cf8151d38e70440484/0
[  118.537382][  T310]
[  118.542693][  T310] job=/lkp/jobs/scheduled/vm-meta-300/boot-1-debian-11.1-x86_64-20220510.cgz-x86_64-randconfig-071-20230903-c6eefad61209-20230903-11038-o17p1l-0.yaml


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20230905/202309051442.bd6f9879-oliver.sang@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v7 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

[Valentine Sinitsyn]

As of now, seeking in sysfs files is handled by generic_file_llseek().
There are situations where one may want to customize seeking logic:

- Many sysfs entries are fixed files while generic_file_llseek() accepts
  past-the-end positions. Not only being useless by itself, this
  also means a bug in userspace code will trigger not at lseek(), but at
  some later point making debugging harder.
- generic_file_llseek() relies on f_mapping->host to get the file size
  which might not be correct for all sysfs entries.
  See commit 636b21b50152 ("PCI: Revoke mappings like devmem") as an example.

Implement llseek method to override this behavior at sysfs attribute
level. The method is optional, and if it is absent,
generic_file_llseek() is called to preserve backwards compatibility.

Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
---
v7:
        - Use proper locking in kernfs_fop_llseek()
v6:
        - Mark pci_llseek_resource() as __maybe_unused
        - Fix a typo in pci_create_legacy_files()
v5:
        - Fix builds without PCI mmap support (e.g. Alpha)
v4:
        - Fix builds which #define HAVE_PCI_LEGACY (e.g. PowerPC)
v3:
        - Grammar fixes
        - Add base-patch: and prerequisite-patch-id: to make kernel test
          robot happy
v2:
        - Add infrastructure to customize llseek per sysfs entry type
        - Override llseek for PCI sysfs entries with fixed_file_llseek()

 fs/kernfs/file.c       | 29 ++++++++++++++++++++++++++++-
 fs/sysfs/file.c        | 13 +++++++++++++
 include/linux/kernfs.h |  1 +
 include/linux/sysfs.h  |  2 ++
 4 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 180906c36f51..1a76fb8128ce 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -903,6 +903,33 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait)
 	return ret;
 }
 
+static loff_t kernfs_fop_llseek(struct file *file, loff_t offset, int whence)
+{
+	struct kernfs_open_file *of = kernfs_of(file);
+	const struct kernfs_ops *ops;
+	loff_t ret;
+
+	/*
+	 * @of->mutex nests outside active ref and is primarily to ensure that
+	 * the ops aren't called concurrently for the same open file.
+	 */
+	mutex_lock(&of->mutex);
+	if (!kernfs_get_active(of->kn)) {
+		mutex_unlock(&of->mutex);
+		return -ENODEV;
+	}
+
+	ops = kernfs_ops(of->kn);
+	if (ops->llseek)
+		ret = ops->llseek(of, offset, whence);
+	else
+		ret = generic_file_llseek(file, offset, whence);
+
+	mutex_unlock(&of->mutex);
+	kernfs_put_active(of->kn);
+	return ret;
+}
+
 static void kernfs_notify_workfn(struct work_struct *work)
 {
 	struct kernfs_node *kn;
@@ -1005,7 +1032,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify);
 const struct file_operations kernfs_file_fops = {
 	.read_iter	= kernfs_fop_read_iter,
 	.write_iter	= kernfs_fop_write_iter,
-	.llseek		= generic_file_llseek,
+	.llseek		= kernfs_fop_llseek,
 	.mmap		= kernfs_fop_mmap,
 	.open		= kernfs_fop_open,
 	.release	= kernfs_fop_release,
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index a12ac0356c69..6b7652fb8050 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -167,6 +167,18 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of,
 	return battr->mmap(of->file, kobj, battr, vma);
 }
 
+static loff_t sysfs_kf_bin_llseek(struct kernfs_open_file *of, loff_t offset,
+				  int whence)
+{
+	struct bin_attribute *battr = of->kn->priv;
+	struct kobject *kobj = of->kn->parent->priv;
+
+	if (battr->llseek)
+		return battr->llseek(of->file, kobj, battr, offset, whence);
+	else
+		return generic_file_llseek(of->file, offset, whence);
+}
+
 static int sysfs_kf_bin_open(struct kernfs_open_file *of)
 {
 	struct bin_attribute *battr = of->kn->priv;
@@ -249,6 +261,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = {
 	.write		= sysfs_kf_bin_write,
 	.mmap		= sysfs_kf_bin_mmap,
 	.open		= sysfs_kf_bin_open,
+	.llseek		= sysfs_kf_bin_llseek,
 };
 
 int sysfs_add_file_mode_ns(struct kernfs_node *parent,
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 2a36f3218b51..99aaa050ccb7 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -316,6 +316,7 @@ struct kernfs_ops {
 			 struct poll_table_struct *pt);
 
 	int (*mmap)(struct kernfs_open_file *of, struct vm_area_struct *vma);
+	loff_t (*llseek)(struct kernfs_open_file *of, loff_t offset, int whence);
 };
 
 /*
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index fd3fe5c8c17f..b717a70219f6 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -181,6 +181,8 @@ struct bin_attribute {
 			char *, loff_t, size_t);
 	ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
 			 char *, loff_t, size_t);
+	loff_t (*llseek)(struct file *, struct kobject *, struct bin_attribute *,
+			 loff_t, int);
 	int (*mmap)(struct file *, struct kobject *, struct bin_attribute *attr,
 		    struct vm_area_struct *vma);
 };

base-commit: a48fa7efaf1161c1c898931fe4c7f0070964233a
-- 
2.34.1

[PATCH v7 2/2] PCI: Implement custom llseek for sysfs resource entries

[Valentine Sinitsyn]

Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"), mmappable
sysfs entries have started to receive their f_mapping from the iomem
pseudo filesystem, so that CONFIG_IO_STRICT_DEVMEM is honored in sysfs
(and procfs) as well as in /dev/[k]mem.

This resulted in a userspace-visible regression:

1. Open a sysfs PCI resource file (eg. /sys/bus/pci/devices/*/resource0)
2. Use lseek(fd, 0, SEEK_END) to determine its size

Expected result: a PCI region size is returned.
Actual result: 0 is returned.

The reason is that PCI resource files residing in sysfs use
generic_file_llseek(), which relies on f_mapping->host inode to get the
file size. As f_mapping is now redefined, f_mapping->host points to an
anonymous zero-sized iomem_inode which has nothing to do with sysfs file
in question.

Implement a custom llseek method for sysfs PCI resources, which is
almost the same as proc_bus_pci_lseek() used for procfs entries.

This makes sysfs and procfs entries consistent with regards to seeking,
but also introduces userspace-visible changes to seeking PCI resources
in sysfs:

- SEEK_DATA and SEEK_HOLE are no longer supported;
- Seeking past the end of the file is prohibited while previously
  offsets up to MAX_NON_LFS were accepted (reading from these offsets
  was always invalid).

Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci-sysfs.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index d9eede2dbc0e..97c9c62d5e3e 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -835,6 +835,19 @@ static const struct attribute_group pci_dev_config_attr_group = {
 	.is_bin_visible = pci_dev_config_attr_is_visible,
 };
 
+/*
+ * llseek operation for mmappable PCI resources.
+ * May be left unused if the arch doesn't provide them.
+ */
+static __maybe_unused loff_t
+pci_llseek_resource(struct file *filep,
+		    struct kobject *kobj __always_unused,
+		    struct bin_attribute *attr,
+		    loff_t offset, int whence)
+{
+	return fixed_size_llseek(filep, offset, whence, attr->size);
+}
+
 #ifdef HAVE_PCI_LEGACY
 /**
  * pci_read_legacy_io - read byte(s) from legacy I/O port space
@@ -967,6 +980,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_io->attr.mode = 0600;
 	b->legacy_io->read = pci_read_legacy_io;
 	b->legacy_io->write = pci_write_legacy_io;
+	/* See pci_create_attr() for motivation */
+	b->legacy_io->llseek = pci_llseek_resource;
 	b->legacy_io->mmap = pci_mmap_legacy_io;
 	b->legacy_io->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_io);
@@ -981,6 +996,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_mem->size = 1024*1024;
 	b->legacy_mem->attr.mode = 0600;
 	b->legacy_mem->mmap = pci_mmap_legacy_mem;
+	/* See pci_create_attr() for motivation */
+	b->legacy_mem->llseek = pci_llseek_resource;
 	b->legacy_mem->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_mem);
 	error = device_create_bin_file(&b->dev, b->legacy_mem);
@@ -1199,8 +1216,15 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
 			res_attr->mmap = pci_mmap_resource_uc;
 		}
 	}
-	if (res_attr->mmap)
+	if (res_attr->mmap) {
 		res_attr->f_mapping = iomem_get_mapping;
+		/*
+		 * generic_file_llseek() consults f_mapping->host to determine
+		 * the file size. As iomem_inode knows nothing about the
+		 * attribute, it's not going to work, so override it as well.
+		 */
+		res_attr->llseek = pci_llseek_resource;
+	}
 	res_attr->attr.name = res_attr_name;
 	res_attr->attr.mode = 0600;
 	res_attr->size = pci_resource_len(pdev, num);
-- 
2.34.1

[PATCH v8 1/2] kernfs: sysfs: support custom llseek method for sysfs entries

[Valentine Sinitsyn]

As of now, seeking in sysfs files is handled by generic_file_llseek().
There are situations where one may want to customize seeking logic:

- Many sysfs entries are fixed files while generic_file_llseek() accepts
  past-the-end positions. Not only being useless by itself, this
  also means a bug in userspace code will trigger not at lseek(), but at
  some later point making debugging harder.
- generic_file_llseek() relies on f_mapping->host to get the file size
  which might not be correct for all sysfs entries.
  See commit 636b21b50152 ("PCI: Revoke mappings like devmem") as an example.

Implement llseek method to override this behavior at sysfs attribute
level. The method is optional, and if it is absent,
generic_file_llseek() is called to preserve backwards compatibility.

Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
----
v8:
        - This is real v7; previous v7 is a buggy earlier patch sent by
          mistake
v7:
        - Use proper locking in kernfs_fop_llseek()
v6:
        - Mark pci_llseek_resource() as __maybe_unused
        - Fix a typo in pci_create_legacy_files()
v5:
        - Fix builds without PCI mmap support (e.g. Alpha)
v4:
        - Fix builds which #define HAVE_PCI_LEGACY (e.g. PowerPC)
v3:
        - Grammar fixes
        - Add base-patch: and prerequisite-patch-id: to make kernel test
          robot happy
v2:
        - Add infrastructure to customize llseek per sysfs entry type
        - Override llseek for PCI sysfs entries with fixed_file_llseek()--

 fs/kernfs/file.c       | 29 ++++++++++++++++++++++++++++-
 fs/sysfs/file.c        | 13 +++++++++++++
 include/linux/kernfs.h |  1 +
 include/linux/sysfs.h  |  2 ++
 4 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 180906c36f51..855e3f9d8dcc 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -903,6 +903,33 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait)
 	return ret;
 }
 
+static loff_t kernfs_fop_llseek(struct file *file, loff_t offset, int whence)
+{
+	struct kernfs_open_file *of = kernfs_of(file);
+	const struct kernfs_ops *ops;
+	loff_t ret;
+
+	/*
+	 * @of->mutex nests outside active ref and is primarily to ensure that
+	 * the ops aren't called concurrently for the same open file.
+	 */
+	mutex_lock(&of->mutex);
+	if (!kernfs_get_active(of->kn)) {
+		mutex_unlock(&of->mutex);
+		return -ENODEV;
+	}
+
+	ops = kernfs_ops(of->kn);
+	if (ops->llseek)
+		ret = ops->llseek(of, offset, whence);
+	else
+		ret = generic_file_llseek(file, offset, whence);
+
+	kernfs_put_active(of->kn);
+	mutex_unlock(&of->mutex);
+	return ret;
+}
+
 static void kernfs_notify_workfn(struct work_struct *work)
 {
 	struct kernfs_node *kn;
@@ -1005,7 +1032,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify);
 const struct file_operations kernfs_file_fops = {
 	.read_iter	= kernfs_fop_read_iter,
 	.write_iter	= kernfs_fop_write_iter,
-	.llseek		= generic_file_llseek,
+	.llseek		= kernfs_fop_llseek,
 	.mmap		= kernfs_fop_mmap,
 	.open		= kernfs_fop_open,
 	.release	= kernfs_fop_release,
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index a12ac0356c69..6b7652fb8050 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -167,6 +167,18 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of,
 	return battr->mmap(of->file, kobj, battr, vma);
 }
 
+static loff_t sysfs_kf_bin_llseek(struct kernfs_open_file *of, loff_t offset,
+				  int whence)
+{
+	struct bin_attribute *battr = of->kn->priv;
+	struct kobject *kobj = of->kn->parent->priv;
+
+	if (battr->llseek)
+		return battr->llseek(of->file, kobj, battr, offset, whence);
+	else
+		return generic_file_llseek(of->file, offset, whence);
+}
+
 static int sysfs_kf_bin_open(struct kernfs_open_file *of)
 {
 	struct bin_attribute *battr = of->kn->priv;
@@ -249,6 +261,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = {
 	.write		= sysfs_kf_bin_write,
 	.mmap		= sysfs_kf_bin_mmap,
 	.open		= sysfs_kf_bin_open,
+	.llseek		= sysfs_kf_bin_llseek,
 };
 
 int sysfs_add_file_mode_ns(struct kernfs_node *parent,
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 2a36f3218b51..99aaa050ccb7 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -316,6 +316,7 @@ struct kernfs_ops {
 			 struct poll_table_struct *pt);
 
 	int (*mmap)(struct kernfs_open_file *of, struct vm_area_struct *vma);
+	loff_t (*llseek)(struct kernfs_open_file *of, loff_t offset, int whence);
 };
 
 /*
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index fd3fe5c8c17f..b717a70219f6 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -181,6 +181,8 @@ struct bin_attribute {
 			char *, loff_t, size_t);
 	ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *,
 			 char *, loff_t, size_t);
+	loff_t (*llseek)(struct file *, struct kobject *, struct bin_attribute *,
+			 loff_t, int);
 	int (*mmap)(struct file *, struct kobject *, struct bin_attribute *attr,
 		    struct vm_area_struct *vma);
 };

base-commit: a48fa7efaf1161c1c898931fe4c7f0070964233a
-- 
2.34.1

[PATCH v8 2/2] PCI: Implement custom llseek for sysfs resource entries

[Valentine Sinitsyn]

Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"), mmappable
sysfs entries have started to receive their f_mapping from the iomem
pseudo filesystem, so that CONFIG_IO_STRICT_DEVMEM is honored in sysfs
(and procfs) as well as in /dev/[k]mem.

This resulted in a userspace-visible regression:

1. Open a sysfs PCI resource file (eg. /sys/bus/pci/devices/*/resource0)
2. Use lseek(fd, 0, SEEK_END) to determine its size

Expected result: a PCI region size is returned.
Actual result: 0 is returned.

The reason is that PCI resource files residing in sysfs use
generic_file_llseek(), which relies on f_mapping->host inode to get the
file size. As f_mapping is now redefined, f_mapping->host points to an
anonymous zero-sized iomem_inode which has nothing to do with sysfs file
in question.

Implement a custom llseek method for sysfs PCI resources, which is
almost the same as proc_bus_pci_lseek() used for procfs entries.

This makes sysfs and procfs entries consistent with regards to seeking,
but also introduces userspace-visible changes to seeking PCI resources
in sysfs:

- SEEK_DATA and SEEK_HOLE are no longer supported;
- Seeking past the end of the file is prohibited while previously
  offsets up to MAX_NON_LFS were accepted (reading from these offsets
  was always invalid).

Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
Cc: stable@vger.kernel.org
Signed-off-by: Valentine Sinitsyn <valesini@yandex-team.ru>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci-sysfs.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index d9eede2dbc0e..97c9c62d5e3e 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -835,6 +835,19 @@ static const struct attribute_group pci_dev_config_attr_group = {
 	.is_bin_visible = pci_dev_config_attr_is_visible,
 };
 
+/*
+ * llseek operation for mmappable PCI resources.
+ * May be left unused if the arch doesn't provide them.
+ */
+static __maybe_unused loff_t
+pci_llseek_resource(struct file *filep,
+		    struct kobject *kobj __always_unused,
+		    struct bin_attribute *attr,
+		    loff_t offset, int whence)
+{
+	return fixed_size_llseek(filep, offset, whence, attr->size);
+}
+
 #ifdef HAVE_PCI_LEGACY
 /**
  * pci_read_legacy_io - read byte(s) from legacy I/O port space
@@ -967,6 +980,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_io->attr.mode = 0600;
 	b->legacy_io->read = pci_read_legacy_io;
 	b->legacy_io->write = pci_write_legacy_io;
+	/* See pci_create_attr() for motivation */
+	b->legacy_io->llseek = pci_llseek_resource;
 	b->legacy_io->mmap = pci_mmap_legacy_io;
 	b->legacy_io->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_io);
@@ -981,6 +996,8 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_mem->size = 1024*1024;
 	b->legacy_mem->attr.mode = 0600;
 	b->legacy_mem->mmap = pci_mmap_legacy_mem;
+	/* See pci_create_attr() for motivation */
+	b->legacy_mem->llseek = pci_llseek_resource;
 	b->legacy_mem->f_mapping = iomem_get_mapping;
 	pci_adjust_legacy_attr(b, pci_mmap_mem);
 	error = device_create_bin_file(&b->dev, b->legacy_mem);
@@ -1199,8 +1216,15 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
 			res_attr->mmap = pci_mmap_resource_uc;
 		}
 	}
-	if (res_attr->mmap)
+	if (res_attr->mmap) {
 		res_attr->f_mapping = iomem_get_mapping;
+		/*
+		 * generic_file_llseek() consults f_mapping->host to determine
+		 * the file size. As iomem_inode knows nothing about the
+		 * attribute, it's not going to work, so override it as well.
+		 */
+		res_attr->llseek = pci_llseek_resource;
+	}
 	res_attr->attr.name = res_attr_name;
 	res_attr->attr.mode = 0600;
 	res_attr->size = pci_resource_len(pdev, num);
-- 
2.34.1