afs: Fix checking wait_event_interruptible() return value

[PATCH] afs: Fix checking wait_event_interruptible() return value

Posted by Alexandra Diupina 2 months, 2 weeks ago

The if-block in question is presumably supposed to handle
wait_event_interruptible() failure, i.e. when it returns -ERESTARTSYS
due to signal received by the task instead of a zero value.

Fix the condition appropriately.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 4be5975aea15 ("afs: Further fix file locking")
Signed-off-by: Alexandra Diupina <adiupina@astralinux.ru>
---
 fs/afs/flock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/afs/flock.c b/fs/afs/flock.c
index f0e96a35093f..2dba323b3395 100644
--- a/fs/afs/flock.c
+++ b/fs/afs/flock.c
@@ -652,7 +652,7 @@ static int afs_do_setlk(struct file *file, struct file_lock *fl)
 			fl->fl_u.afs.state = AFS_LOCK_PENDING;
 			goto try_to_lock;
 		case AFS_LOCK_PENDING:
-			if (ret > 0) {
+			if (ret < 0) {
 				/* We need to retry the lock.  We may not be
 				 * notified by the server if it just expired
 				 * rather than being released.
-- 
2.30.2

Re: [PATCH] afs: Fix checking wait_event_interruptible() return value

Posted by David Howells 2 months, 2 weeks ago

Alexandra Diupina <adiupina@astralinux.ru> wrote:

> The if-block in question is presumably supposed to handle
> wait_event_interruptible() failure, i.e. when it returns -ERESTARTSYS
> due to signal received by the task instead of a zero value.
> 
> Fix the condition appropriately.
> ...
> -			if (ret > 0) {
> +			if (ret < 0) {

This isn't correct.  If we take a signal, we need to drop out immediately
rather than going round again.  However, you're right and "ret > 0" won't ever
happen.  I need to have a ponder on this.

David