linux-kernel - Re: [RFC v2 08/16] luo: luo_files: add infrastructure for FDs

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADrL8HWdAzQRVhLh2Pq1GV2fy6tus66Eo37PvG9A98kUJTihsg@mail.gmail.com>
Date: Thu, 15 May 2025 16:15:36 -0700
From: James Houghton <jthoughton@...gle.com>
To: Pasha Tatashin <pasha.tatashin@...een.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
Subject: Re: [RFC v2 08/16] luo: luo_files: add infrastructure for FDs

On Thu, May 15, 2025 at 11:23 AM Pasha Tatashin
<pasha.tatashin@...een.com> wrote:
> +/**
> + * luo_retrieve_file - Find a registered file instance by its token.
> + * @token: The unique token of the file instance to retrieve.
> + * @file: Output parameter. On success (return value 0), this will point
> + * to the retrieved "struct file".
> + *
> + * Searches the global list for a &struct luo_file matching the @token. Uses a
> + * read lock, allowing concurrent retrievals.
> + *
> + * Return: 0 on success. Negative errno on failure.
> + */
> +int luo_retrieve_file(u64 token, struct file **file)
> +{
> +       struct luo_file *luo_file;
> +       int ret = 0;
> +
> +       luo_files_recreate_luo_files_xa_in();
> +       luo_state_read_enter();
> +       if (!liveupdate_state_updated()) {
> +               pr_warn("File can be retrieved only in updated state\n");
> +               luo_state_read_exit();
> +               return -EBUSY;
> +       }
> +
> +       luo_file = xa_load(&luo_files_xa_in, token);
> +       if (luo_file && !luo_file->reclaimed) {
> +               luo_file->reclaimed = true;

I haven't been able to pay too much attention to the series yet, and I
know this was posted as an RFC, so pardon my nit-picking.

I think you need to have xchg here for this not to be racy, so something like:

`if (luo_file && !xchg(&luo_file->reclaimed, true))`

Or maybe you meant to avoid this race some other way; IIUC,
luo_state_read_enter() is not sufficient.

Thanks!

> +               ret = luo_file->fs->retrieve(luo_file->fs->arg,
> +                                            luo_file->private_data,
> +                                            file);
> +               if (!ret)
> +                       luo_file->file = *file;
> +       } else if (luo_file && luo_file->reclaimed) {
> +               pr_err("The file descriptor for token %lld has already been retrieved\n",
> +                      token);
> +               ret = -EINVAL;
> +       } else {
> +               ret = -ENOENT;
> +       }
> +
> +       luo_state_read_exit();
> +
> +       return ret;
> +}