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]
Date: Mon, 29 Jan 2024 14:23:17 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: "Fabio M. De Francesco" <fabio.maria.de.francesco@...ux.intel.com>,
	<linux-kernel@...r.kernel.org>
CC: "Fabio M. De Francesco" <fabio.maria.de.francesco@...ux.intel.com>, "Peter
 Zijlstra" <peterz@...radead.org>, Dan Williams <dan.j.williams@...el.com>,
	"Ira Weiny" <ira.weiny@...el.com>
Subject: Re: [RFC PATCH] cleanup: Add cond_guard() to conditional guards

Fabio M. De Francesco wrote:
> Add cond_guard() to conditional guards.
> 
> cond_guard() stores the return value from _interruptible(), _killable(),
> and _try versions of locks to a variable whose address is passed as the
> second parameter, so that the value can later be checked and an action
> be taken accordingly (e.g., calling printk() in case of failure).

This does not look like it returns the result from the conditional
locking function, it looks like it returns whether the boolean of
whether the CLASS() constructor succeeded or failed:

 "*_ret = (!__guard_ptr(_name)(&scope)"

Even if it can't return the result from the lock call itself directly,
it would still be nice to be able to write something like:

    rc = cond_guard(rwsem_read_intr, -EINTR, &rwsem);
    if (rc)
        return rc;

..and know that the lock is taken of -EINTR is returned.

That would also mean being able to write "if (cond_guard())", which I
don't think would work with the current proposal.

> As the other guards, it avoids to open code the release of the lock
> after a goto to an 'out' label.
> 
> This is an RFC at least for two reasons...
> 
> 1) I'm not sure that this guard is needed unless we want to avoid the
> use of scoped_cond_guard() with its necessary indentation of the code
> in the successful path and/or we want to check the return value of the
> conditional lock.

Taking this argument to its logical conclusion would mean removing plain
guard() as well. There is definitely a code readability benefit for
having the option of guard() vs scoped_guard(), so it would be nice to
have similar flexibility in the conditional case.

> 2) By using cond_guard() it is not possible to release the lock before
> calling other functions from the current one. The lock is held until the
> current function exits. This is not always desirable.

No, that's not correct, the lock is only held until the exit from the
current scope, and if you call functions from within that scope the lock
remains held. So:

    func1()
    {
        guard(...)(lock1);
        if (condition) {
            guard(...)(lock2);
            func2();
        }
        func3();
    }

func2() is called with lock1 and lock2 held, func3() is only called with
lock1() held.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ