[PATCH 0/2] Couple of efivarfs fixes

[PATCH 0/2] Couple of efivarfs fixes

[Tony Luck]

1) Some apps want to monitor changes in EFI variables, but reading the
   file and comparing is inefficient.  Just have Linnux update the
   modification time when a file is written

2) A rate limited read can return -EINTR ... very suprising to apps.

Tony Luck (2):
  efivarfs: Update inode modification time for successful writes
  efivarfs: Don't return -EINTR when rate-limiting reads

 fs/efivarfs/file.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

-- 
2.21.1

[PATCH 1/2] efivarfs: Update inode modification time for successful writes

[Tony Luck]

Some applications want to be able to see when EFI variables
have been updated.

Update the modification time for successful writes.

Reported-by: Lennart Poettering <mzxreary@0pointer.de>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 fs/efivarfs/file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index e9e27a271af0..4b8bc4560d70 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -51,6 +51,7 @@ static ssize_t efivarfs_file_write(struct file *file,
 	} else {
 		inode_lock(inode);
 		i_size_write(inode, datasize + sizeof(attributes));
+		inode->i_mtime = current_time(inode);
 		inode_unlock(inode);
 	}
 
-- 
2.21.1

[PATCH 2/2] efivarfs: Don't return -EINTR when rate-limiting reads

[Tony Luck]

Applications that read EFI variables may see a return
value of -EINTR if they exceed the rate limit and a
signal delivery is attempted while the process is sleeping.

This is quite surprising to the application, which probably
doesn't have code to handle it.

Change the interruptible sleep to a non-interruptible one.

Reported-by: Lennart Poettering <mzxreary@0pointer.de>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 fs/efivarfs/file.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index 4b8bc4560d70..feaa5e182b7b 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -73,10 +73,8 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
 	ssize_t size = 0;
 	int err;
 
-	while (!__ratelimit(&file->f_cred->user->ratelimit)) {
-		if (!msleep_interruptible(50))
-			return -EINTR;
-	}
+	while (!__ratelimit(&file->f_cred->user->ratelimit))
+		msleep(50);
 
 	err = efivar_entry_size(var, &datasize);
 
-- 
2.21.1

Re: [PATCH 0/2] Couple of efivarfs fixes

[Ard Biesheuvel]

On Thu, 28 May 2020 at 21:49, Tony Luck <tony.luck@intel.com> wrote:
>
> 1) Some apps want to monitor changes in EFI variables, but reading the
>    file and comparing is inefficient.  Just have Linnux update the
>    modification time when a file is written
>
> 2) A rate limited read can return -EINTR ... very suprising to apps.
>
> Tony Luck (2):
>   efivarfs: Update inode modification time for successful writes
>   efivarfs: Don't return -EINTR when rate-limiting reads
>
>  fs/efivarfs/file.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>

Queued in efi/urgent

Thanks,

[tip: efi/urgent] efivarfs: Update inode modification time for successful writes

[tip-bot2 for Tony Luck]

The following commit has been merged into the efi/urgent branch of tip:

Commit-ID:     2096721f1577b51b574fa06a7d91823dffe7267a
Gitweb:        https://git.kernel.org/tip/2096721f1577b51b574fa06a7d91823dffe7267a
Author:        Tony Luck <tony.luck@intel.com>
AuthorDate:    Thu, 28 May 2020 12:49:04 -07:00
Committer:     Ard Biesheuvel <ardb@kernel.org>
CommitterDate: Mon, 15 Jun 2020 14:38:56 +02:00

efivarfs: Update inode modification time for successful writes

Some applications want to be able to see when EFI variables
have been updated.

Update the modification time for successful writes.

Reported-by: Lennart Poettering <mzxreary@0pointer.de>
Signed-off-by: Tony Luck <tony.luck@intel.com>
Link: https://lore.kernel.org/r/20200528194905.690-2-tony.luck@intel.com
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 fs/efivarfs/file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index e9e27a2..4b8bc45 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -51,6 +51,7 @@ static ssize_t efivarfs_file_write(struct file *file,
 	} else {
 		inode_lock(inode);
 		i_size_write(inode, datasize + sizeof(attributes));
+		inode->i_mtime = current_time(inode);
 		inode_unlock(inode);
 	}
 

[tip: efi/urgent] efivarfs: Don't return -EINTR when rate-limiting reads

[tip-bot2 for Tony Luck]

The following commit has been merged into the efi/urgent branch of tip:

Commit-ID:     4353f03317fd3eb0bd803b61bdb287b68736a728
Gitweb:        https://git.kernel.org/tip/4353f03317fd3eb0bd803b61bdb287b68736a728
Author:        Tony Luck <tony.luck@intel.com>
AuthorDate:    Thu, 28 May 2020 12:49:05 -07:00
Committer:     Ard Biesheuvel <ardb@kernel.org>
CommitterDate: Mon, 15 Jun 2020 14:38:56 +02:00

efivarfs: Don't return -EINTR when rate-limiting reads

Applications that read EFI variables may see a return
value of -EINTR if they exceed the rate limit and a
signal delivery is attempted while the process is sleeping.

This is quite surprising to the application, which probably
doesn't have code to handle it.

Change the interruptible sleep to a non-interruptible one.

Reported-by: Lennart Poettering <mzxreary@0pointer.de>
Signed-off-by: Tony Luck <tony.luck@intel.com>
Link: https://lore.kernel.org/r/20200528194905.690-3-tony.luck@intel.com
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 fs/efivarfs/file.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index 4b8bc45..feaa5e1 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -73,10 +73,8 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
 	ssize_t size = 0;
 	int err;
 
-	while (!__ratelimit(&file->f_cred->user->ratelimit)) {
-		if (!msleep_interruptible(50))
-			return -EINTR;
-	}
+	while (!__ratelimit(&file->f_cred->user->ratelimit))
+		msleep(50);
 
 	err = efivar_entry_size(var, &datasize);