Received-SPF: pass (google.com: domain of linux-kernel+bounces-3831-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20231218141645.2548743-1-alpic@google.com>
In-Reply-To: <20231218141645.2548743-1-alpic@google.com>
From: Alfred Piccioni <alpic@google.com>
Date: Mon, 18 Dec 2023 15:22:50 +0100
Message-ID: <CALcwBGDoEjyfAnYHaCCqULa+dwRyw3spHijUXwJ_LAQp=oV-pg@mail.gmail.com>
Subject: Re: [PATCH] SELinux: Introduce security_file_ioctl_compat hook
To: Paul Moore <paul@paul-moore.com>, Stephen Smalley <stephen.smalley.work@gmail.com>, 
	Eric Paris <eparis@parisplace.org>
Cc: stable@vger.kernel.org, selinux@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"

> s/emmits/emits/

Fixed.

> It doesn't (or shouldn't) replace security_file_ioctl, and the hook
> doesn't appear to be conditional on CONFIG_COMPAT per se.
> It is a new hook that is called from the compat ioctl syscall. The old
> hook continues to be used from the regular ioctl syscall and
> elsewhere.

Yup, reworded to be more correct. Partially lack of understanding on
my part of how the ioctl syscalls were being made.

> I don't understand why you made this change, possibly a leftover of an
> earlier version of the patch that tried to replace
> security_file_ioctl() everywhere?

Correct. Forgot to go back and remove it. Fixed.

> By the way, for extra credit, you could augment the ioctl tests in the
> selinux-testsuite to also exercise this new hook and confirm that it
> works correctly. See
> https://github.com/SELinuxProject/selinux-testsuite particularly
> tests/ioctl and policy/test_ioctl.te. Feel free to ask for help on
> that.

I do like extra credit. I'll take a look and see if it's something I
can tackle. I'm primarily doing ad hoc checks on Android devices, so
I'm unsure how easy it will be for me to run the suite. I'll get back
to you shortly on that.