Re: [PATCH bpf 5/7] bpf: Change the type of unsafe_ptr in bpf_iter_bits_new()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/9/2024 10:45 AM, Hou Tao wrote:
>
> On 10/9/2024 2:30 AM, Andrii Nakryiko wrote:
>> On Tue, Oct 8, 2024 at 2:05 AM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote:
>>> From: Hou Tao <houtao1@xxxxxxxxxx>
>>>
>>> Under 32-bits host (e.g, arm32) , when a bpf program passes an u64 to
>>> bpf_iter_bits_new(), bpf_iter_bits_new() will use bits_copy to save the
>>> content of the u64, but the size of bits_copy is only 4-bytes, and there
>>> will be stack corruption.
>>>
>>> Fix it by change the type of unsafe_ptr from u64 * to unsigned long *.
>>>
>> This will be confusing as BPF-side long is always 64-bit. So why not
>> instead make sure it's u64 throughout (i.e., bits_copy is u64
>> explicitly), even on 32-bit architectures?
> Just learn about the size of BPF-side long is always 64-bits. I had
> considered to change bits_copy to u64. The main obstacle is that the
> pointer type of find_next_bit is unsigned long *, if it is used on an
> u64 under big-endian host, it may return invalid result.

I think doing the following swap for big endian and 32-bits host will
let find_next_bit return the correct result:

+static void swap_bits(u64 *bits, unsigned int nr)
+{
+#if defined(__BIG_ENDIAN) && !defined(CONFIG_64BIT)
+       unsigned int i;
+
+       for (i = 0; i < nr; i++)
+               bits[i] = (bits[i] >> 32) | ((u64)(u32)bits[i] << 32);
+#endif
+}
+

Will try to get some test environment to test it.
>>> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
>>> ---
>>>  kernel/bpf/helpers.c | 18 ++++++++++--------
>>>  1 file changed, 10 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>>> index 6c0205d5018c..dee69c3904a0 100644
>>> --- a/kernel/bpf/helpers.c
>>> +++ b/kernel/bpf/helpers.c
>>> @@ -2852,7 +2852,7 @@ struct bpf_iter_bits {
>>>  } __aligned(8);
>>>
>>>  /* nr_bits only has 31 bits */
>>> -#define BITS_ITER_NR_WORDS_MAX ((1U << 31) / BITS_PER_TYPE(u64))
>>> +#define BITS_ITER_NR_WORDS_MAX ((1U << 31) / BITS_PER_TYPE(unsigned long))
>>>
>>>  struct bpf_iter_bits_kern {
>>>         union {
>>> @@ -2868,8 +2868,9 @@ struct bpf_iter_bits_kern {
>>>   * bpf_iter_bits_new() - Initialize a new bits iterator for a given memory area
>>>   * @it: The new bpf_iter_bits to be created
>>>   * @unsafe_ptr__ign: A pointer pointing to a memory area to be iterated over
>>> - * @nr_words: The size of the specified memory area, measured in 8-byte units.
>>> - * Due to the limitation of memalloc, it can't be greater than 512.
>>> + * @nr_words: The size of the specified memory area, measured in units of
>>> + * sizeof(unsigned long). Due to the limitation of memalloc, it can't be
>>> + * greater than 512.
>>>   *
>>>   * This function initializes a new bpf_iter_bits structure for iterating over
>>>   * a memory area which is specified by the @unsafe_ptr__ign and @nr_words. It
>>> @@ -2879,17 +2880,18 @@ struct bpf_iter_bits_kern {
>>>   * On success, 0 is returned. On failure, ERR is returned.
>>>   */
>>>  __bpf_kfunc int
>>> -bpf_iter_bits_new(struct bpf_iter_bits *it, const u64 *unsafe_ptr__ign, u32 nr_words)
>>> +bpf_iter_bits_new(struct bpf_iter_bits *it, const unsigned long *unsafe_ptr__ign, u32 nr_words)
>>>  {
>>> -       struct bpf_iter_bits_kern *kit = (void *)it;
>>> -       u32 nr_bytes = nr_words * sizeof(u64);
>>> +       u32 nr_bytes = nr_words * sizeof(*unsafe_ptr__ign);
>>>         u32 nr_bits = BYTES_TO_BITS(nr_bytes);
>>> +       struct bpf_iter_bits_kern *kit;
>>>         int err;
>>>
>>>         BUILD_BUG_ON(sizeof(struct bpf_iter_bits_kern) != sizeof(struct bpf_iter_bits));
>>>         BUILD_BUG_ON(__alignof__(struct bpf_iter_bits_kern) !=
>>>                      __alignof__(struct bpf_iter_bits));
>>>
>>> +       kit = (void *)it;
>>>         kit->allocated = 0;
>>>         kit->nr_bits = 0;
>>>         kit->bits_copy = 0;
>>> @@ -2900,8 +2902,8 @@ bpf_iter_bits_new(struct bpf_iter_bits *it, const u64 *unsafe_ptr__ign, u32 nr_w
>>>         if (nr_words > BITS_ITER_NR_WORDS_MAX)
>>>                 return -E2BIG;
>>>
>>> -       /* Optimization for u64 mask */
>>> -       if (nr_bits == 64) {
>>> +       /* Optimization for unsigned long mask */
>>> +       if (nr_words == 1) {
>>>                 err = bpf_probe_read_kernel_common(&kit->bits_copy, nr_bytes, unsafe_ptr__ign);
>>>                 if (err)
>>>                         return -EFAULT;
>>> --
>>> 2.29.2
>>>
>> .
> .





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux