| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CA+CK2bB+NRneE=uFxvQ867zT3BHGvfBUz+6Ntqk9p2=wj4JYWQ@mail.gmail.com>
Date: Mon, 22 Sep 2025 19:17:31 -0400
From: Pasha Tatashin <pasha.tatashin@...een.com>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: pratyush@...nel.org, jasonmiu@...gle.com, graf@...zon.com,
changyuanl@...gle.com, rppt@...nel.org, dmatlack@...gle.com,
rientjes@...gle.com, corbet@....net, rdunlap@...radead.org,
ilpo.jarvinen@...ux.intel.com, kanie@...ux.alibaba.com, ojeda@...nel.org,
aliceryhl@...gle.com, masahiroy@...nel.org, akpm@...ux-foundation.org,
tj@...nel.org, yoann.congal@...le.fr, mmaurer@...gle.com,
roman.gushchin@...ux.dev, chenridong@...wei.com, axboe@...nel.dk,
mark.rutland@....com, jannh@...gle.com, vincent.guittot@...aro.org,
hannes@...xchg.org, dan.j.williams@...el.com, david@...hat.com,
joel.granados@...nel.org, rostedt@...dmis.org, anna.schumaker@...cle.com,
song@...nel.org, zhangguopeng@...inos.cn, linux@...ssschuh.net,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, linux-mm@...ck.org,
gregkh@...uxfoundation.org, tglx@...utronix.de, mingo@...hat.com,
bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
rafael@...nel.org, dakr@...nel.org, bartosz.golaszewski@...aro.org,
cw00.choi@...sung.com, myungjoo.ham@...sung.com, yesanishhere@...il.com,
Jonathan.Cameron@...wei.com, quic_zijuhu@...cinc.com,
aleksander.lobakin@...el.com, ira.weiny@...el.com,
andriy.shevchenko@...ux.intel.com, leon@...nel.org, lukas@...ner.de,
bhelgaas@...gle.com, wagi@...nel.org, djeffery@...hat.com,
stuart.w.hayes@...il.com, ptyadav@...zon.de, lennart@...ttering.net,
brauner@...nel.org, linux-api@...r.kernel.org, linux-fsdevel@...r.kernel.org,
saeedm@...dia.com, ajayachandra@...dia.com, parav@...dia.com,
leonro@...dia.com, witu@...dia.com
Subject: Re: [PATCH v3 18/30] liveupdate: luo_files: luo_ioctl: Add ioctls for
per-file state management
On Thu, Aug 14, 2025 at 10:02 AM Jason Gunthorpe <jgg@...dia.com> wrote:
>
> On Thu, Aug 07, 2025 at 01:44:24AM +0000, Pasha Tatashin wrote:
> > +struct liveupdate_ioctl_get_fd_state {
> > + __u32 size;
> > + __u8 incoming;
> > + __aligned_u64 token;
> > + __u32 state;
> > +};
>
> Same remark about explicit padding and checking padding for 0
Done
> > + * luo_file_get_state - Get the preservation state of a specific file.
> > + * @token: The token of the file to query.
> > + * @statep: Output pointer to store the file's current live update state.
> > + * @incoming: If true, query the state of a restored file from the incoming
> > + * (previous kernel's) set. If false, query a file being prepared
> > + * for preservation in the current set.
> > + *
> > + * Finds the file associated with the given @token in either the incoming
> > + * or outgoing tracking arrays and returns its current LUO state
> > + * (NORMAL, PREPARED, FROZEN, UPDATED).
> > + *
> > + * Return: 0 on success, -ENOENT if the token is not found.
> > + */
> > +int luo_file_get_state(u64 token, enum liveupdate_state *statep, bool incoming)
> > +{
> > + struct luo_file *luo_file;
> > + struct xarray *target_xa;
> > + int ret = 0;
> > +
> > + luo_state_read_enter();
>
> Less globals, at this point everything should be within memory
> attached to the file descriptor and not in globals. Doing this will
> promote good maintainable structure and not a spaghetti
>
> Also I think a BKL design is not a good idea for new code. We've had
> so many bad experiences with this pattern promoting uncontrolled
> incomprehensible locking.
>
> The xarray already has a lock, why not have reasonable locking inside
> the luo_file? Probably just a refcount?
>
> > + target_xa = incoming ? &luo_files_xa_in : &luo_files_xa_out;
> > + luo_file = xa_load(target_xa, token);
> > +
> > + if (!luo_file) {
> > + ret = -ENOENT;
> > + goto out_unlock;
> > + }
> > +
> > + scoped_guard(mutex, &luo_file->mutex)
> > + *statep = luo_file->state;
> > +
> > +out_unlock:
> > + luo_state_read_exit();
>
> If we are using cleanup.h then use it for this too..
>
> But it seems kind of weird, why not just
>
> xa_lock()
> xa_load()
> *statep = READ_ONCE(luo_file->state);
> xa_unlock()
>
> ?
Yes, we can simplify with xa_lock(), thank you for your suggestion.
>
> > +static int luo_ioctl_set_fd_event(struct luo_ucmd *ucmd)
> > +{
> > + struct liveupdate_ioctl_set_fd_event *argp = ucmd->cmd;
> > + int ret;
> > +
> > + switch (argp->event) {
> > + case LIVEUPDATE_PREPARE:
> > + ret = luo_file_prepare(argp->token);
> > + break;
> > + case LIVEUPDATE_FREEZE:
> > + ret = luo_file_freeze(argp->token);
> > + break;
> > + case LIVEUPDATE_FINISH:
> > + ret = luo_file_finish(argp->token);
> > + break;
> > + case LIVEUPDATE_CANCEL:
> > + ret = luo_file_cancel(argp->token);
> > + break;
>
> The token should be converted to a file here instead of duplicated in
> each function
struct luo_file is preivate to luo_file.c, and I think it makes sense
to keep it that way, amount of duplicated code is trivial.00
> > static int luo_open(struct inode *inodep, struct file *filep)
> > {
> > if (atomic_cmpxchg(&luo_device_in_use, 0, 1))
> > @@ -149,6 +191,8 @@ union ucmd_buffer {
> > struct liveupdate_ioctl_fd_restore restore;
> > struct liveupdate_ioctl_get_state state;
> > struct liveupdate_ioctl_set_event event;
> > + struct liveupdate_ioctl_get_fd_state fd_state;
> > + struct liveupdate_ioctl_set_fd_event fd_event;
> > };
> >
> > struct luo_ioctl_op {
> > @@ -179,6 +223,10 @@ static const struct luo_ioctl_op luo_ioctl_ops[] = {
> > struct liveupdate_ioctl_get_state, state),
> > IOCTL_OP(LIVEUPDATE_IOCTL_SET_EVENT, luo_ioctl_set_event,
> > struct liveupdate_ioctl_set_event, event),
> > + IOCTL_OP(LIVEUPDATE_IOCTL_GET_FD_STATE, luo_ioctl_get_fd_state,
> > + struct liveupdate_ioctl_get_fd_state, token),
> > + IOCTL_OP(LIVEUPDATE_IOCTL_SET_FD_EVENT, luo_ioctl_set_fd_event,
> > + struct liveupdate_ioctl_set_fd_event, token),
> > };
>
> Keep sorted
Done
Powered by blists - more mailing lists