Re: [PATCH] drm/i915: Fix CFI violations in gt_sysfs

From: Nathan Chancellor <nathan@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Tom Rix <trix@redhat.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	llvm@lists.linux.dev, patches@lists.linux.dev
Subject: Re: [PATCH] drm/i915: Fix CFI violations in gt_sysfs
Date: Sat, 24 Sep 2022 21:39:30 -0700	[thread overview]
Message-ID: <Yy/bglnYjnFfjDDI@dev-arch.thelio-3990X> (raw)
In-Reply-To: <202209232143.172BCCDC4@keescook>

On Fri, Sep 23, 2022 at 09:57:47PM -0700, Kees Cook wrote:
> On Thu, Sep 22, 2022 at 12:51:27PM -0700, Nathan Chancellor wrote:
> > [...]
> > To make everything work properly, adjust certain functions to match the
> > type of the ->show() and ->store() members in 'struct kobj_attribute'.
> > Add a macro to generate functions for that can be called via both
> > dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be
> > called through both kobject locations without violating kCFI and adjust
> > the attribute groups to account for this.
> 
> This was quite a roller coaster! I think the solution looks good, even
> if I'm suspicious of the original design that has the same stuff
> available twice in different places. (I have a dim memory of rdma
> needing a refactoring like this too?)

Right, I noticed this comment in intel_gt_sysfs_register() once I fully
saw what was going on:

	/*
	 * We need to make things right with the
	 * ABI compatibility. The files were originally
	 * generated under the parent directory.
	 *
	 * We generate the files only for gt 0
	 * to avoid duplicates.
	 */

Makes it seem like there will be userspace breakage if these files do
not exist? I figured this was the cleanest solution within those
parameters.

> Reviewed-by: Kees Cook <keescook@chromium.org>

Thanks for looking it over!

Cheers,
Nathan