On Mon, May 11, 2020 at 05:59:04PM -0400, Rafael Aquini wrote:
> The sysctl knob allows any user with SYS_ADMIN capability to
> taint the kernel with any arbitrary value, but this might
> produce an invalid flags bitset being committed to tainted_mask.
>
> This patch introduces a simple way for proc_taint() to ignore
> any eventual invalid bit coming from the user input before
> committing those bits to the kernel tainted_mask, as well as
> it makes clear use of TAINT_USER flag to mark the kernel
> tainted by user everytime a taint value is written
> to the kernel.tainted sysctl.
>
> Signed-off-by: Rafael Aquini <aquini@xxxxxxxxxx>
> ---
> kernel/sysctl.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 8a176d8727a3..f0a4fb38ac62 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2623,17 +2623,32 @@ static int proc_taint(struct ctl_table *table, int write,
> return err;
>
> if (write) {
> + int i;
> +
> + /*
> + * Ignore user input that would make us committing
> + * arbitrary invalid TAINT flags in the loop below.
> + */
> + tmptaint &= (1UL << TAINT_FLAGS_COUNT) - 1;
This looks good but we don't pr_warn() of information lost on intention.
> +
> /*
> * Poor man's atomic or. Not worth adding a primitive
> * to everyone's atomic.h for this
> */
> - int i;
> for (i = 0; i < BITS_PER_LONG && tmptaint >> i; i++) {
> if ((tmptaint >> i) & 1)
> add_taint(i, LOCKDEP_STILL_OK);
> }
> +
> + /*
> + * Users with SYS_ADMIN capability can include any arbitrary
> + * taint flag by writing to this interface. If that's the case,
> + * we also need to mark the kernel "tainted by user".
> + */
> + add_taint(TAINT_USER, LOCKDEP_STILL_OK);
I'm in favor of this however I'd like to hear from Ted on if it meets
the original intention. I would think he had a good reason not to add
it here.
Luis
> }
>
> +
> return err;
> }
>
> --
> 2.25.4
>
