netdev - Re: [PATCH 0/3] using guard/_

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: <0e7af4cb0dc19be7cc0267256284a70ceb250b38.camel@sipsolutions.net>
Date: Wed, 27 Mar 2024 22:28:33 +0100
From: Johannes Berg <johannes@...solutions.net>
To: Jakub Sitnicki <jakub@...udflare.com>
Cc: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org, Peter Zijlstra
	 <peterz@...radead.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] using guard/__free in networking

On Wed, 2024-03-27 at 21:25 +0100, Jakub Sitnicki wrote:
> On Tue, Mar 26, 2024 at 04:33 PM +01, Johannes Berg wrote:
> > > Is it also present in Rust or some such?
> > 
> > I have no idea. I _think_ Rust actually ties the data and the locks
> > together more?
> 
> That is right. Nicely explained here:
> 
> https://marabos.nl/atomics/basics.html#rusts-mutex

Right.

Thinking about that, we _could_ even add support for drop_guard()?

With the below patch to cleanup.h, you can write

void my_something(my_t *my)
{
...
	named_guard(lock, mutex)(&my->mutex);
...
	if (foo)
		return -EINVAL; // automatically unlocks
...
	// no need for lock any more
	drop_guard(lock);
...
	// do other things now unlocked
}


Is that too ugly? Maybe it's less Java-like and more Rust-like and
better for Jakub ;-)

In some sense that's nicer than scoped_guard() since it doesn't require
a new scope / indentation, but maybe Peter already thought about it and
rejected it :-)


Patch follows, though maybe that should be rolled into the 'base' CLASS
definition instead of defining another "_drop" one for named_guard()?

diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index c2d09bc4f976..31298106c28b 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -106,7 +106,27 @@ typedef _type class_##_name##_t;					\
 static inline void class_##_name##_destructor(_type *p)			\
 { _type _T = *p; _exit; }						\
 static inline _type class_##_name##_constructor(_init_args)		\
-{ _type t = _init; return t; }
+{ _type t = _init; return t; }						\
+typedef struct class_##_name##_drop##_t {				\
+	class_##_name##_t obj;						\
+	void (*destructor)(struct class_##_name##_drop##_t *);		\
+} class_##_name##_drop##_t;						\
+static inline void							\
+class_##_name##_drop##_destructor(class_##_name##_drop##_t *p)		\
+{									\
+	if (p->obj)							\
+		class_##_name##_destructor(&p->obj);			\
+	p->obj = NULL;							\
+}									\
+static inline class_##_name##_drop##_t					\
+class_##_name##_drop##_constructor(_init_args)				\
+{									\
+	class_##_name##_drop##_t t = {					\
+		.obj = _init,						\
+		.destructor = class_##_name##_drop##_destructor,	\
+	};								\
+	return t;							\
+}
 
 #define EXTEND_CLASS(_name, ext, _init, _init_args...)			\
 typedef class_##_name##_t class_##_name##ext##_t;			\
@@ -163,6 +183,11 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
 #define guard(_name) \
 	CLASS(_name, __UNIQUE_ID(guard))
 
+#define named_guard(_name, _class) \
+	CLASS(_class##_drop, _name)
+
+#define drop_guard(_name) do { _name.destructor(&_name); } while (0)
+
 #define __guard_ptr(_name) class_##_name##_lock_ptr
 
 #define scoped_guard(_name, args...)					\