linux-kernel - Re: [PATCH 01/12] drm/i915: Indicate which pipe failed the fastset check overall

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: <1013ff2d-76b2-41f9-a5d4-39a567a3b0cc@rasmusvillemoes.dk>
Date: Tue, 27 Feb 2024 10:38:10 +0100
From: Rasmus Villemoes <linux@...musvillemoes.dk>
To: Jani Nikula <jani.nikula@...ux.intel.com>,
 Ville Syrjälä <ville.syrjala@...ux.intel.com>,
 Rodrigo Vivi <rodrigo.vivi@...el.com>
Cc: intel-gfx@...ts.freedesktop.org, Petr Mladek <pmladek@...e.com>,
 Steven Rostedt <rostedt@...dmis.org>,
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Sergey Senozhatsky <senozhatsky@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/12] drm/i915: Indicate which pipe failed the fastset
 check overall

On 26/02/2024 15.57, Jani Nikula wrote:

> Personally I suck at remembering even the standard printf conversion
> specifiers, let alone all the kernel extensions. I basically have to
> look them up every time. I'd really love some %{name} format for named
> pointer things. And indeed preferrably without the %p. Just %{name}.

Sorry to spoil the fun, but that's a non-starter.

foo.c: In function ‘foo’:
foo.c:5:24: warning: unknown conversion type character ‘{’ in format
[-Wformat=]
    5 |         printf("Hello %{function} World\n", &foo);
      |                        ^

You can't start accepting stuff that -Wformat will warn about. We're not
going to start building with Wno-format.

> And then we could discuss adding support for drm specific things. I
> guess one downside is that the functions to do this would have to be in
> vsprintf.c instead of drm. Unless we add some code in drm for this
> that's always built-in.

If people can be trusted to write callbacks with the proper semantics
for snprintf [1], we could do a generic

typedef char * (*printf_callback)(char *buf, char *end, void *ctx);

struct printf_ext {
  printf_callback cb;
  void *ctx;
};

#define PRINTF_EXT(callback, context) &(struct printf_ext){ .cb =
callback, .ctx = context }

// in drm-land

char* my_drm_gizmo_formatter(char *buf, char *end, void *ctx)
{
  struct drm_gizmo *dg = ctx;
  ....
  return buf;
}
#define pX_gizmo(dg) PRINTF_EXT(my_drm_gizmo_formatter, dg)

   printk("error: gizmo %pX in wrong state!\n", pX_gizmo(dg));

Then vsprintf.c doesn't need to know anything about any particular
subsystem. And if a subsystem breaks snprintf semantics, they get to
keep the pieces. With a little more macro magic, one might even be able
to throw in some type safety checks.

Rasmus

[1] You can't sleep, you can't allocate memory, you probably can't even
take any raw spinlocks, you must attempt to do the full formatting so
you can tell how much room would be needed, but you must of course not
write anything beyond end. Calling vsnprintf() to format various integer
members is probably ok, but recursively using %pX to print full
subobjects is likely a bad idea.