[PATCH 2/3] liveupdate: keep sessions retrievable after release failures

Leo Timmins posted 3 patches 1 week, 4 days ago
There is a newer version of this series
[PATCH 2/3] liveupdate: keep sessions retrievable after release failures
Posted by Leo Timmins 1 week, 4 days ago
When automatic finish-on-release fails for an incoming session,
luo_session_release() cannot report that error back through close(). The
VFS ignores ->release() return values during __fput().

Returning with session->retrieved still set leaves the session on the
incoming list but impossible to retrieve again after the file descriptor
is gone. Clear the retrieved flag so userspace can reopen the session and
retry finishing it later.

Fixes: 16cec0d26521 ("liveupdate: luo_session: add ioctls for file preservation")

Signed-off-by: Leo Timmins <leotimmins1974@gmail.com>
---
 kernel/liveupdate/luo_session.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/liveupdate/luo_session.c b/kernel/liveupdate/luo_session.c
index 25ae704d7787..7016d96ee960 100644
--- a/kernel/liveupdate/luo_session.c
+++ b/kernel/liveupdate/luo_session.c
@@ -210,9 +210,11 @@ static int luo_session_release(struct inode *inodep, struct file *filep)
 		int err = luo_session_finish_one(session);
 
 		if (err) {
+			scoped_guard(mutex, &session->mutex)
+				session->retrieved = false;
 			pr_warn("Unable to finish session [%s] on release\n",
 				session->name);
-			return err;
+			return 0;
 		}
 		sh = &luo_session_global.incoming;
 	} else {
-- 
2.53.0
Re: [PATCH 2/3] liveupdate: keep sessions retrievable after release failures
Posted by Pasha Tatashin 1 week, 4 days ago
Hi Leo,

Sorry for dup, accidentally switched to HTML mode in previous reply,
and therefore it was rejected from the mailing lists.

On Tue, Mar 24, 2026 at 2:03 PM Leo Timmins <leotimmins1974@gmail.com> wrote:
>
> When automatic finish-on-release fails for an incoming session,
> luo_session_release() cannot report that error back through close(). The
> VFS ignores ->release() return values during __fput().

Exactly. Since userspace can't detect a close() failure without
parsing dmesg, we shouldn't design around it. The API gives explicit
ioctl(LIVEUPDATE_SESSION_FINISH) that user should use before closing
the session, and if needed repeate finish again.

If userspace closes the session fd before finishing, we have to treat
it as a userspace bug. While it does mean the resource leaks until the
next live update or reboot, trying to work around this by allowing
session retrieval after a close() doesn't make sense. There is no
clean way for userspace to know the session is even still there;
perhaps blindly firing ioctl(LIVEUPDATE_SESSION_RETRIEVE_FD) would
allow querying that.

> Returning with session->retrieved still set leaves the session on the
> incoming list but impossible to retrieve again after the file descriptor
> is gone. Clear the retrieved flag so userspace can reopen the session and
> retry finishing it later.
>
> Fixes: 16cec0d26521 ("liveupdate: luo_session: add ioctls for file preservation")

It changes functionality, not really sure that Fixes applies.

> Signed-off-by: Leo Timmins <leotimmins1974@gmail.com>
> ---
>  kernel/liveupdate/luo_session.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/liveupdate/luo_session.c b/kernel/liveupdate/luo_session.c
> index 25ae704d7787..7016d96ee960 100644
> --- a/kernel/liveupdate/luo_session.c
> +++ b/kernel/liveupdate/luo_session.c
> @@ -210,9 +210,11 @@ static int luo_session_release(struct inode *inodep, struct file *filep)
>                 int err = luo_session_finish_one(session);
>
>                 if (err) {
> +                       scoped_guard(mutex, &session->mutex)
> +                               session->retrieved = false;
>                         pr_warn("Unable to finish session [%s] on release\n",
>                                 session->name);
> -                       return err;
> +                       return 0;

close() do not return failure to userspace, but is there a
disadvantage in propagating this up?

Thanks,
Pasha

>                 }
>                 sh = &luo_session_global.incoming;
>         } else {
> --
> 2.53.0