* [PATCH bpf 0/1] bpf: btf: Fix endianness of bitfields
@ 2018-07-09 0:22 Okash Khawaja
2018-07-09 0:22 ` [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian Okash Khawaja
0 siblings, 1 reply; 10+ messages in thread
From: Okash Khawaja @ 2018-07-09 0:22 UTC (permalink / raw)
To: Daniel Borkmann, Martin KaFai Lau, Alexei Starovoitov,
Yonghong Song, Jakub Kicinski, David S. Miller
Cc: netdev, kernel-team, linux-kernel
Hi,
This patch fixes endianness bug in btf_int_bits_seq_show(). Jakub
Kicinski pointed out in a separate patch review for bpftool ("bpf: btf:
add btf print functionality") that parsing of bitfield might not work
on big endian machine. Similar parsing is performed in
btf_int_bits_seq_show() and this patch fixes it.
Thanks,
Okash
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian 2018-07-09 0:22 [PATCH bpf 0/1] bpf: btf: Fix endianness of bitfields Okash Khawaja @ 2018-07-09 0:22 ` Okash Khawaja 2018-07-09 17:32 ` Martin KaFai Lau 2018-07-09 18:32 ` Martin KaFai Lau 0 siblings, 2 replies; 10+ messages in thread From: Okash Khawaja @ 2018-07-09 0:22 UTC (permalink / raw) To: Daniel Borkmann, Martin KaFai Lau, Alexei Starovoitov, Yonghong Song, Jakub Kicinski, David S. Miller Cc: netdev, kernel-team, linux-kernel [-- Attachment #1: fix-btf-bitfields.patch --] [-- Type: text/plain, Size: 2568 bytes --] When extracting bitfield from a number, btf_int_bits_seq_show() builds a mask and accesses least significant byte of the number in a way specific to little-endian. This patch fixes that by checking endianness of the machine and then shifting left and right the unneeded bits. Thanks to Martin Lau for the help in navigating potential pitfalls when dealing with endianess and for the final solution. Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info") Signed-off-by: Okash Khawaja <osk@fb.com> --- kernel/bpf/btf.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -162,6 +162,8 @@ #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3) #define BITS_ROUNDUP_BYTES(bits) \ (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits)) +const int one = 1; +#define is_big_endian() ((*(char *)&one) == 0) #define BTF_INFO_MASK 0x0f00ffff #define BTF_INT_MASK 0x0fffffff @@ -991,16 +993,13 @@ static void btf_int_bits_seq_show(const void *data, u8 bits_offset, struct seq_file *m) { + u8 left_shift_bits, right_shift_bits; u32 int_data = btf_type_int(t); u16 nr_bits = BTF_INT_BITS(int_data); u16 total_bits_offset; u16 nr_copy_bytes; u16 nr_copy_bits; - u8 nr_upper_bits; - union { - u64 u64_num; - u8 u8_nums[8]; - } print_num; + u64 print_num; total_bits_offset = bits_offset + BTF_INT_OFFSET(int_data); data += BITS_ROUNDDOWN_BYTES(total_bits_offset); @@ -1008,21 +1007,20 @@ static void btf_int_bits_seq_show(const nr_copy_bits = nr_bits + bits_offset; nr_copy_bytes = BITS_ROUNDUP_BYTES(nr_copy_bits); - print_num.u64_num = 0; - memcpy(&print_num.u64_num, data, nr_copy_bytes); - - /* Ditch the higher order bits */ - nr_upper_bits = BITS_PER_BYTE_MASKED(nr_copy_bits); - if (nr_upper_bits) { - /* We need to mask out some bits of the upper byte. */ - u8 mask = (1 << nr_upper_bits) - 1; - - print_num.u8_nums[nr_copy_bytes - 1] &= mask; + print_num = 0; + memcpy(&print_num, data, nr_copy_bytes); + if (is_big_endian()) { + left_shift_bits = bits_offset; + right_shift_bits = BITS_PER_U64 - nr_bits; + } else { + left_shift_bits = BITS_PER_U64 - nr_copy_bits; + right_shift_bits = BITS_PER_U64 - nr_bits; } - print_num.u64_num >>= bits_offset; + print_num <<= left_shift_bits; + print_num >>= right_shift_bits; - seq_printf(m, "0x%llx", print_num.u64_num); + seq_printf(m, "0x%llx", print_num); } static void btf_int_seq_show(const struct btf *btf, const struct btf_type *t, ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian 2018-07-09 0:22 ` [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian Okash Khawaja @ 2018-07-09 17:32 ` Martin KaFai Lau 2018-07-09 18:32 ` Martin KaFai Lau 1 sibling, 0 replies; 10+ messages in thread From: Martin KaFai Lau @ 2018-07-09 17:32 UTC (permalink / raw) To: Okash Khawaja Cc: Daniel Borkmann, Alexei Starovoitov, Yonghong Song, Jakub Kicinski, David S. Miller, netdev, kernel-team, linux-kernel On Sun, Jul 08, 2018 at 05:22:03PM -0700, Okash Khawaja wrote: > When extracting bitfield from a number, btf_int_bits_seq_show() builds > a mask and accesses least significant byte of the number in a way > specific to little-endian. This patch fixes that by checking endianness > of the machine and then shifting left and right the unneeded bits. > > Thanks to Martin Lau for the help in navigating potential pitfalls when > dealing with endianess and for the final solution. > > Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info") > Signed-off-by: Okash Khawaja <osk@fb.com> > > --- > kernel/bpf/btf.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -162,6 +162,8 @@ > #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3) > #define BITS_ROUNDUP_BYTES(bits) \ > (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits)) > +const int one = 1; > +#define is_big_endian() ((*(char *)&one) == 0) Can the __BIG_ENDIAN be reused here? > > #define BTF_INFO_MASK 0x0f00ffff > #define BTF_INT_MASK 0x0fffffff > @@ -991,16 +993,13 @@ static void btf_int_bits_seq_show(const > void *data, u8 bits_offset, > struct seq_file *m) > { > + u8 left_shift_bits, right_shift_bits; > u32 int_data = btf_type_int(t); > u16 nr_bits = BTF_INT_BITS(int_data); > u16 total_bits_offset; > u16 nr_copy_bytes; > u16 nr_copy_bits; > - u8 nr_upper_bits; > - union { > - u64 u64_num; > - u8 u8_nums[8]; > - } print_num; > + u64 print_num; > > total_bits_offset = bits_offset + BTF_INT_OFFSET(int_data); > data += BITS_ROUNDDOWN_BYTES(total_bits_offset); > @@ -1008,21 +1007,20 @@ static void btf_int_bits_seq_show(const > nr_copy_bits = nr_bits + bits_offset; > nr_copy_bytes = BITS_ROUNDUP_BYTES(nr_copy_bits); > > - print_num.u64_num = 0; > - memcpy(&print_num.u64_num, data, nr_copy_bytes); > - > - /* Ditch the higher order bits */ > - nr_upper_bits = BITS_PER_BYTE_MASKED(nr_copy_bits); > - if (nr_upper_bits) { > - /* We need to mask out some bits of the upper byte. */ > - u8 mask = (1 << nr_upper_bits) - 1; > - > - print_num.u8_nums[nr_copy_bytes - 1] &= mask; > + print_num = 0; > + memcpy(&print_num, data, nr_copy_bytes); > + if (is_big_endian()) { > + left_shift_bits = bits_offset; > + right_shift_bits = BITS_PER_U64 - nr_bits; > + } else { > + left_shift_bits = BITS_PER_U64 - nr_copy_bits; > + right_shift_bits = BITS_PER_U64 - nr_bits; > } > > - print_num.u64_num >>= bits_offset; > + print_num <<= left_shift_bits; > + print_num >>= right_shift_bits; > > - seq_printf(m, "0x%llx", print_num.u64_num); > + seq_printf(m, "0x%llx", print_num); > } > > static void btf_int_seq_show(const struct btf *btf, const struct btf_type *t, > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian 2018-07-09 0:22 ` [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian Okash Khawaja 2018-07-09 17:32 ` Martin KaFai Lau @ 2018-07-09 18:32 ` Martin KaFai Lau 2018-07-10 8:21 ` Daniel Borkmann 2018-07-10 16:35 ` David Laight 1 sibling, 2 replies; 10+ messages in thread From: Martin KaFai Lau @ 2018-07-09 18:32 UTC (permalink / raw) To: Okash Khawaja Cc: Daniel Borkmann, Alexei Starovoitov, Yonghong Song, Jakub Kicinski, David S. Miller, netdev, kernel-team, linux-kernel On Sun, Jul 08, 2018 at 05:22:03PM -0700, Okash Khawaja wrote: > When extracting bitfield from a number, btf_int_bits_seq_show() builds > a mask and accesses least significant byte of the number in a way > specific to little-endian. This patch fixes that by checking endianness > of the machine and then shifting left and right the unneeded bits. > > Thanks to Martin Lau for the help in navigating potential pitfalls when > dealing with endianess and for the final solution. > > Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info") > Signed-off-by: Okash Khawaja <osk@fb.com> > > --- > kernel/bpf/btf.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -162,6 +162,8 @@ > #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3) > #define BITS_ROUNDUP_BYTES(bits) \ > (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits)) > +const int one = 1; > +#define is_big_endian() ((*(char *)&one) == 0) > > #define BTF_INFO_MASK 0x0f00ffff > #define BTF_INT_MASK 0x0fffffff > @@ -991,16 +993,13 @@ static void btf_int_bits_seq_show(const > void *data, u8 bits_offset, > struct seq_file *m) > { > + u8 left_shift_bits, right_shift_bits; Nit. Although only max 64 bit int is allowed now (ensured by btf_int_check_meta), it is better to use u16 such that it will be consistent to BTF_INT_BITS. > u32 int_data = btf_type_int(t); > u16 nr_bits = BTF_INT_BITS(int_data); > u16 total_bits_offset; > u16 nr_copy_bytes; > u16 nr_copy_bits; > - u8 nr_upper_bits; > - union { > - u64 u64_num; > - u8 u8_nums[8]; > - } print_num; > + u64 print_num; > > total_bits_offset = bits_offset + BTF_INT_OFFSET(int_data); > data += BITS_ROUNDDOWN_BYTES(total_bits_offset); > @@ -1008,21 +1007,20 @@ static void btf_int_bits_seq_show(const > nr_copy_bits = nr_bits + bits_offset; > nr_copy_bytes = BITS_ROUNDUP_BYTES(nr_copy_bits); > > - print_num.u64_num = 0; > - memcpy(&print_num.u64_num, data, nr_copy_bytes); > - > - /* Ditch the higher order bits */ > - nr_upper_bits = BITS_PER_BYTE_MASKED(nr_copy_bits); > - if (nr_upper_bits) { > - /* We need to mask out some bits of the upper byte. */ > - u8 mask = (1 << nr_upper_bits) - 1; > - > - print_num.u8_nums[nr_copy_bytes - 1] &= mask; > + print_num = 0; > + memcpy(&print_num, data, nr_copy_bytes); > + if (is_big_endian()) { > + left_shift_bits = bits_offset; > + right_shift_bits = BITS_PER_U64 - nr_bits; > + } else { > + left_shift_bits = BITS_PER_U64 - nr_copy_bits; > + right_shift_bits = BITS_PER_U64 - nr_bits; Nit. right_shift_bits is the same for both cases. Lets simplify it. > } > > - print_num.u64_num >>= bits_offset; > + print_num <<= left_shift_bits; > + print_num >>= right_shift_bits; > > - seq_printf(m, "0x%llx", print_num.u64_num); > + seq_printf(m, "0x%llx", print_num); > } > > static void btf_int_seq_show(const struct btf *btf, const struct btf_type *t, > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian 2018-07-09 18:32 ` Martin KaFai Lau @ 2018-07-10 8:21 ` Daniel Borkmann 2018-07-10 17:49 ` Okash Khawaja 2018-07-10 16:35 ` David Laight 1 sibling, 1 reply; 10+ messages in thread From: Daniel Borkmann @ 2018-07-10 8:21 UTC (permalink / raw) To: Martin KaFai Lau, Okash Khawaja Cc: Alexei Starovoitov, Yonghong Song, Jakub Kicinski, David S. Miller, netdev, kernel-team, linux-kernel On 07/09/2018 08:32 PM, Martin KaFai Lau wrote: > On Sun, Jul 08, 2018 at 05:22:03PM -0700, Okash Khawaja wrote: >> When extracting bitfield from a number, btf_int_bits_seq_show() builds >> a mask and accesses least significant byte of the number in a way >> specific to little-endian. This patch fixes that by checking endianness >> of the machine and then shifting left and right the unneeded bits. >> >> Thanks to Martin Lau for the help in navigating potential pitfalls when >> dealing with endianess and for the final solution. >> >> Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info") >> Signed-off-by: Okash Khawaja <osk@fb.com> >> >> --- >> kernel/bpf/btf.c | 32 +++++++++++++++----------------- >> 1 file changed, 15 insertions(+), 17 deletions(-) >> >> --- a/kernel/bpf/btf.c >> +++ b/kernel/bpf/btf.c >> @@ -162,6 +162,8 @@ >> #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3) >> #define BITS_ROUNDUP_BYTES(bits) \ >> (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits)) >> +const int one = 1; >> +#define is_big_endian() ((*(char *)&one) == 0) Also here, in the kernel archs provide proper definitions. >> #define BTF_INFO_MASK 0x0f00ffff >> #define BTF_INT_MASK 0x0fffffff >> @@ -991,16 +993,13 @@ static void btf_int_bits_seq_show(const >> void *data, u8 bits_offset, >> struct seq_file *m) >> { >> + u8 left_shift_bits, right_shift_bits; > Nit. > Although only max 64 bit int is allowed now (ensured by btf_int_check_meta), > it is better to use u16 such that it will be consistent to BTF_INT_BITS. > >> u32 int_data = btf_type_int(t); >> u16 nr_bits = BTF_INT_BITS(int_data); >> u16 total_bits_offset; >> u16 nr_copy_bytes; >> u16 nr_copy_bits; >> - u8 nr_upper_bits; >> - union { >> - u64 u64_num; >> - u8 u8_nums[8]; >> - } print_num; >> + u64 print_num; >> >> total_bits_offset = bits_offset + BTF_INT_OFFSET(int_data); >> data += BITS_ROUNDDOWN_BYTES(total_bits_offset); >> @@ -1008,21 +1007,20 @@ static void btf_int_bits_seq_show(const >> nr_copy_bits = nr_bits + bits_offset; >> nr_copy_bytes = BITS_ROUNDUP_BYTES(nr_copy_bits); >> >> - print_num.u64_num = 0; >> - memcpy(&print_num.u64_num, data, nr_copy_bytes); >> - >> - /* Ditch the higher order bits */ >> - nr_upper_bits = BITS_PER_BYTE_MASKED(nr_copy_bits); >> - if (nr_upper_bits) { >> - /* We need to mask out some bits of the upper byte. */ >> - u8 mask = (1 << nr_upper_bits) - 1; >> - >> - print_num.u8_nums[nr_copy_bytes - 1] &= mask; >> + print_num = 0; >> + memcpy(&print_num, data, nr_copy_bytes); >> + if (is_big_endian()) { >> + left_shift_bits = bits_offset; >> + right_shift_bits = BITS_PER_U64 - nr_bits; >> + } else { >> + left_shift_bits = BITS_PER_U64 - nr_copy_bits; >> + right_shift_bits = BITS_PER_U64 - nr_bits; > Nit. > right_shift_bits is the same for both cases. Lets simplify it. > >> } >> >> - print_num.u64_num >>= bits_offset; >> + print_num <<= left_shift_bits; >> + print_num >>= right_shift_bits; >> >> - seq_printf(m, "0x%llx", print_num.u64_num); >> + seq_printf(m, "0x%llx", print_num); >> } >> >> static void btf_int_seq_show(const struct btf *btf, const struct btf_type *t, >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian 2018-07-10 8:21 ` Daniel Borkmann @ 2018-07-10 17:49 ` Okash Khawaja 2018-07-10 20:02 ` Daniel Borkmann 0 siblings, 1 reply; 10+ messages in thread From: Okash Khawaja @ 2018-07-10 17:49 UTC (permalink / raw) To: Daniel Borkmann Cc: Martin KaFai Lau, Alexei Starovoitov, Yonghong Song, Jakub Kicinski, David S. Miller, netdev, kernel-team, linux-kernel On Tue, Jul 10, 2018 at 10:21:02AM +0200, Daniel Borkmann wrote: > On 07/09/2018 08:32 PM, Martin KaFai Lau wrote: > > On Sun, Jul 08, 2018 at 05:22:03PM -0700, Okash Khawaja wrote: > >> When extracting bitfield from a number, btf_int_bits_seq_show() builds > >> a mask and accesses least significant byte of the number in a way > >> specific to little-endian. This patch fixes that by checking endianness > >> of the machine and then shifting left and right the unneeded bits. > >> > >> Thanks to Martin Lau for the help in navigating potential pitfalls when > >> dealing with endianess and for the final solution. > >> > >> Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info") > >> Signed-off-by: Okash Khawaja <osk@fb.com> > >> > >> --- > >> kernel/bpf/btf.c | 32 +++++++++++++++----------------- > >> 1 file changed, 15 insertions(+), 17 deletions(-) > >> > >> --- a/kernel/bpf/btf.c > >> +++ b/kernel/bpf/btf.c > >> @@ -162,6 +162,8 @@ > >> #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3) > >> #define BITS_ROUNDUP_BYTES(bits) \ > >> (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits)) > >> +const int one = 1; > >> +#define is_big_endian() ((*(char *)&one) == 0) > > Also here, in the kernel archs provide proper definitions. Is this the __BIG_ENDIAN #define or are there better ways to check that? Thanks, Okash ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian 2018-07-10 17:49 ` Okash Khawaja @ 2018-07-10 20:02 ` Daniel Borkmann 0 siblings, 0 replies; 10+ messages in thread From: Daniel Borkmann @ 2018-07-10 20:02 UTC (permalink / raw) To: Okash Khawaja Cc: Martin KaFai Lau, Alexei Starovoitov, Yonghong Song, Jakub Kicinski, David S. Miller, netdev, kernel-team, linux-kernel On 07/10/2018 07:49 PM, Okash Khawaja wrote: > On Tue, Jul 10, 2018 at 10:21:02AM +0200, Daniel Borkmann wrote: >> On 07/09/2018 08:32 PM, Martin KaFai Lau wrote: >>> On Sun, Jul 08, 2018 at 05:22:03PM -0700, Okash Khawaja wrote: >>>> When extracting bitfield from a number, btf_int_bits_seq_show() builds >>>> a mask and accesses least significant byte of the number in a way >>>> specific to little-endian. This patch fixes that by checking endianness >>>> of the machine and then shifting left and right the unneeded bits. >>>> >>>> Thanks to Martin Lau for the help in navigating potential pitfalls when >>>> dealing with endianess and for the final solution. >>>> >>>> Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info") >>>> Signed-off-by: Okash Khawaja <osk@fb.com> >>>> >>>> --- >>>> kernel/bpf/btf.c | 32 +++++++++++++++----------------- >>>> 1 file changed, 15 insertions(+), 17 deletions(-) >>>> >>>> --- a/kernel/bpf/btf.c >>>> +++ b/kernel/bpf/btf.c >>>> @@ -162,6 +162,8 @@ >>>> #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3) >>>> #define BITS_ROUNDUP_BYTES(bits) \ >>>> (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits)) >>>> +const int one = 1; >>>> +#define is_big_endian() ((*(char *)&one) == 0) >> >> Also here, in the kernel archs provide proper definitions. > Is this the __BIG_ENDIAN #define or are there better ways to check that? Given this deals with bitfields, should be __{BIG,LITTLE}_ENDIAN_BITFIELD. ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian 2018-07-09 18:32 ` Martin KaFai Lau 2018-07-10 8:21 ` Daniel Borkmann @ 2018-07-10 16:35 ` David Laight 2018-07-10 17:18 ` Martin KaFai Lau 1 sibling, 1 reply; 10+ messages in thread From: David Laight @ 2018-07-10 16:35 UTC (permalink / raw) To: 'Martin KaFai Lau', Okash Khawaja Cc: Daniel Borkmann, Alexei Starovoitov, Yonghong Song, Jakub Kicinski, David S. Miller, netdev@vger.kernel.org, kernel-team@fb.com, linux-kernel@vger.kernel.org From: Martin KaFai Lau > Sent: 09 July 2018 19:33 > On Sun, Jul 08, 2018 at 05:22:03PM -0700, Okash Khawaja wrote: > > When extracting bitfield from a number, btf_int_bits_seq_show() builds > > a mask and accesses least significant byte of the number in a way > > specific to little-endian. This patch fixes that by checking endianness > > of the machine and then shifting left and right the unneeded bits. > > > > Thanks to Martin Lau for the help in navigating potential pitfalls when > > dealing with endianess and for the final solution. > > > > Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info") > > Signed-off-by: Okash Khawaja <osk@fb.com> > > > > --- > > kernel/bpf/btf.c | 32 +++++++++++++++----------------- > > 1 file changed, 15 insertions(+), 17 deletions(-) > > > > --- a/kernel/bpf/btf.c > > +++ b/kernel/bpf/btf.c > > @@ -162,6 +162,8 @@ > > #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3) > > #define BITS_ROUNDUP_BYTES(bits) \ > > (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits)) > > +const int one = 1; > > +#define is_big_endian() ((*(char *)&one) == 0) > > > > #define BTF_INFO_MASK 0x0f00ffff > > #define BTF_INT_MASK 0x0fffffff > > @@ -991,16 +993,13 @@ static void btf_int_bits_seq_show(const > > void *data, u8 bits_offset, > > struct seq_file *m) > > { > > + u8 left_shift_bits, right_shift_bits; > Nit. > Although only max 64 bit int is allowed now (ensured by btf_int_check_meta), > it is better to use u16 such that it will be consistent to BTF_INT_BITS. Double-nit. Use 'int' or 'unsigned int'. Sub-word arithmetic will require extra instructions on almost everything except x86. David ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian 2018-07-10 16:35 ` David Laight @ 2018-07-10 17:18 ` Martin KaFai Lau 2018-07-10 18:13 ` Daniel Borkmann 0 siblings, 1 reply; 10+ messages in thread From: Martin KaFai Lau @ 2018-07-10 17:18 UTC (permalink / raw) To: David Laight Cc: Okash Khawaja, Daniel Borkmann, Alexei Starovoitov, Yonghong Song, Jakub Kicinski, David S. Miller, netdev@vger.kernel.org, kernel-team@fb.com, linux-kernel@vger.kernel.org On Tue, Jul 10, 2018 at 04:35:04PM +0000, David Laight wrote: > From: Martin KaFai Lau > > Sent: 09 July 2018 19:33 > > On Sun, Jul 08, 2018 at 05:22:03PM -0700, Okash Khawaja wrote: > > > When extracting bitfield from a number, btf_int_bits_seq_show() builds > > > a mask and accesses least significant byte of the number in a way > > > specific to little-endian. This patch fixes that by checking endianness > > > of the machine and then shifting left and right the unneeded bits. > > > > > > Thanks to Martin Lau for the help in navigating potential pitfalls when > > > dealing with endianess and for the final solution. > > > > > > Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info") > > > Signed-off-by: Okash Khawaja <osk@fb.com> > > > > > > --- > > > kernel/bpf/btf.c | 32 +++++++++++++++----------------- > > > 1 file changed, 15 insertions(+), 17 deletions(-) > > > > > > --- a/kernel/bpf/btf.c > > > +++ b/kernel/bpf/btf.c > > > @@ -162,6 +162,8 @@ > > > #define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3) > > > #define BITS_ROUNDUP_BYTES(bits) \ > > > (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits)) > > > +const int one = 1; > > > +#define is_big_endian() ((*(char *)&one) == 0) > > > > > > #define BTF_INFO_MASK 0x0f00ffff > > > #define BTF_INT_MASK 0x0fffffff > > > @@ -991,16 +993,13 @@ static void btf_int_bits_seq_show(const > > > void *data, u8 bits_offset, > > > struct seq_file *m) > > > { > > > + u8 left_shift_bits, right_shift_bits; > > Nit. > > Although only max 64 bit int is allowed now (ensured by btf_int_check_meta), > > it is better to use u16 such that it will be consistent to BTF_INT_BITS. > > Double-nit. > > Use 'int' or 'unsigned int'. > Sub-word arithmetic will require extra instructions on almost everything > except x86. I would prefer to keep it as u16 which is the max width that is allowed for this field in the wire format. Keeping the usage consistent can avoid accidentally incorrect offsetting or writing wrong data out in other cases. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian 2018-07-10 17:18 ` Martin KaFai Lau @ 2018-07-10 18:13 ` Daniel Borkmann 0 siblings, 0 replies; 10+ messages in thread From: Daniel Borkmann @ 2018-07-10 18:13 UTC (permalink / raw) To: Martin KaFai Lau, David Laight Cc: Okash Khawaja, Alexei Starovoitov, Yonghong Song, Jakub Kicinski, David S. Miller, netdev@vger.kernel.org, kernel-team@fb.com, linux-kernel@vger.kernel.org On 07/10/2018 07:18 PM, Martin KaFai Lau wrote: [...] > I would prefer to keep it as u16 which is the max width that is allowed for > this field in the wire format. Keeping the usage consistent can avoid > accidentally incorrect offsetting or writing wrong data out in other > cases. +1 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-07-10 20:02 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-07-09 0:22 [PATCH bpf 0/1] bpf: btf: Fix endianness of bitfields Okash Khawaja 2018-07-09 0:22 ` [PATCH bpf 1/1] bpf: btf: Fix bitfield extraction for big endian Okash Khawaja 2018-07-09 17:32 ` Martin KaFai Lau 2018-07-09 18:32 ` Martin KaFai Lau 2018-07-10 8:21 ` Daniel Borkmann 2018-07-10 17:49 ` Okash Khawaja 2018-07-10 20:02 ` Daniel Borkmann 2018-07-10 16:35 ` David Laight 2018-07-10 17:18 ` Martin KaFai Lau 2018-07-10 18:13 ` Daniel Borkmann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.