| Message ID | 42a8061a-b626-443a-ad42-0e05b043c6c7@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for-4.19?] libelf: avoid UB in elf_xen_feature_{get,set}() | expand |
On 20/06/2024 4:34 pm, Jan Beulich wrote: > When the left shift amount is up to 31, the shifted quantity wants to be > of unsigned int (or wider) type. > > While there also adjust types: get doesn't alter the array and returns a > boolean, while both don't really accept negative "nr". Drop a stray > blank each as well. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> +1 for 4.19. > --- > Really I wonder why these exist at all; they're effectively test_bit() > and __set_bit() in hypervisor terms, and iirc something like that exists > in the tool stack as well. The toolstack has tools/libs/ctrl/xc_bitops.h but they're not API compatible with Xen. They're long-granular rather than int-granular, have swapped arguments, and are non-LOCKed. ~Andrew
On Thu, 2024-06-20 at 17:07 +0100, Andrew Cooper wrote: > On 20/06/2024 4:34 pm, Jan Beulich wrote: > > When the left shift amount is up to 31, the shifted quantity wants > > to be > > of unsigned int (or wider) type. > > > > While there also adjust types: get doesn't alter the array and > > returns a > > boolean, while both don't really accept negative "nr". Drop a stray > > blank each as well. > > > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > > +1 for 4.19. Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com> ~ Oleksii > > > --- > > Really I wonder why these exist at all; they're effectively > > test_bit() > > and __set_bit() in hypervisor terms, and iirc something like that > > exists > > in the tool stack as well. > > The toolstack has tools/libs/ctrl/xc_bitops.h but they're not API > compatible with Xen. > > They're long-granular rather than int-granular, have swapped > arguments, > and are non-LOCKed. > > ~Andrew
--- a/xen/include/xen/libelf.h +++ b/xen/include/xen/libelf.h @@ -445,13 +445,13 @@ struct elf_dom_parms { uint64_t virt_kend; }; -static inline void elf_xen_feature_set(int nr, uint32_t * addr) +static inline void elf_xen_feature_set(unsigned int nr, uint32_t *addr) { - addr[nr >> 5] |= 1 << (nr & 31); + addr[nr >> 5] |= 1U << (nr & 31); } -static inline int elf_xen_feature_get(int nr, uint32_t * addr) +static inline bool elf_xen_feature_get(unsigned int nr, const uint32_t *addr) { - return !!(addr[nr >> 5] & (1 << (nr & 31))); + return addr[nr >> 5] & (1U << (nr & 31)); } elf_errorstatus elf_xen_parse_features(const char *features,
When the left shift amount is up to 31, the shifted quantity wants to be of unsigned int (or wider) type. While there also adjust types: get doesn't alter the array and returns a boolean, while both don't really accept negative "nr". Drop a stray blank each as well. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Really I wonder why these exist at all; they're effectively test_bit() and __set_bit() in hypervisor terms, and iirc something like that exists in the tool stack as well.