Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   David Laight <David.Laight@ACULAB.COM>
To:     'Josh Triplett' <josh@joshtriplett.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] prctl: Add PR_GET_AUXV to copy auxv to userspace
Thread-Topic: [PATCH v2] prctl: Add PR_GET_AUXV to copy auxv to userspace
Thread-Index: AQHZZvF0xTrvFiiVXUaEsGS9R+Yyu68chzVQ
Date:   Wed, 5 Apr 2023 10:42:03 +0000
Message-ID: <795caad1048c4ac2b831b321f9efe9d6@AcuMS.aculab.com>
References: <d81864a7f7f43bca6afa2a09fc2e850e4050ab42.1680611394.git.josh@joshtriplett.org>
In-Reply-To: <d81864a7f7f43bca6afa2a09fc2e850e4050ab42.1680611394.git.josh@joshtriplett.org>
Accept-Language: en-GB, en-US
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

From: Josh Triplett
> Sent: 04 April 2023 13:32
> 
> If a library wants to get information from auxv (for instance,
> AT_HWCAP/AT_HWCAP2), it has a few options, none of them perfectly
> reliable or ideal:
> 
> - Be main or the pre-main startup code, and grub through the stack above
>   main. Doesn't work for a library.
> - Call libc getauxval. Not ideal for libraries that are trying to be
>   libc-independent and/or don't otherwise require anything from other
>   libraries.
> - Open and read /proc/self/auxv. Doesn't work for libraries that may run
>   in arbitrarily constrained environments that may not have /proc
>   mounted (e.g. libraries that might be used by an init program or a
>   container setup tool).
> - Assume you're on the main thread and still on the original stack, and
>   try to walk the stack upwards, hoping to find auxv. Extremely bad
>   idea.
> - Ask the caller to pass auxv in for you. Not ideal for a user-friendly
>   library, and then your caller may have the same problem.
> 
> Add a prctl that copies current->mm->saved_auxv to a userspace buffer.
...
> +static int prctl_get_auxv(void __user *addr, unsigned long len)
> +{
> +	struct mm_struct *mm = current->mm;
> +	unsigned long size = min_t(unsigned long, sizeof(mm->saved_auxv), len);

Don't use min_t() fix the types.
min_t() is a horrid abomination that is massively overused.

It would be better to have a min_unchecked() that just skips the
type test.

Or accept my patches that allows allow min/max against
compile-time constants between 0 and MAX_INT.
After all, the only reason for the type check is to try
to avoid negative values becoming large positive ones
due to integer promotions.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)