Re: [PATCH] block: avoid sign extend problem with default queue flags mask

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

 



On Fri, Sep 30, 2022 at 6:03 PM Brian Foster <bfoster@xxxxxxxxxx> wrote:
>
> request_queue->queue_flags is an 8-byte field. Most queue flag
> modifications occur through bit field helpers, but default flags can
> be logically OR'd via the QUEUE_FLAG_MQ_DEFAULT mask. If this mask
> happens to include bit 31, the assignment can sign extend the field
> and set all upper 32 bits.
>
> This exact problem has been observed on a downstream kernel that
> happens to use bit 31 for QUEUE_FLAG_NOWAIT. This is not an
> immediate problem for current upstream because bit 31 is not
> included in the default flag assignment (and is not used at all,
> actually). Regardless, fix up the QUEUE_FLAG_MQ_DEFAULT mask
> definition to avoid the landmine in the future.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
>
> Just to elaborate, I ran a quick test to change QUEUE_FLAG_NOWAIT to use
> bit 31. With that change but without this patch, I see the following
> queue state:
>
> # cat /sys/kernel/debug/block/vda/state
> SAME_COMP|IO_STAT|INIT_DONE|WC|STATS|REGISTERED|30|NOWAIT|32|33|34|35|36|37|38|39|40|41|42|43|44|45|46|47|48|49|50|51|52|53|54|55|56|57|58|59|60|61|62|63
>
> And then with the patch applied:
>
> # cat /sys/kernel/debug/block/vda/state
> SAME_COMP|IO_STAT|INIT_DONE|WC|STATS|REGISTERED|30|NOWAIT
>
> Thanks.
>
> Brian
>
>  include/linux/blkdev.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 84b13fdd34a7..28c3037cb25c 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -580,9 +580,9 @@ struct request_queue {
>  #define QUEUE_FLAG_NOWAIT       29     /* device supports NOWAIT */
>  #define QUEUE_FLAG_SQ_SCHED     30     /* single queue style io dispatch */
>
> -#define QUEUE_FLAG_MQ_DEFAULT  ((1 << QUEUE_FLAG_IO_STAT) |            \
> -                                (1 << QUEUE_FLAG_SAME_COMP) |          \
> -                                (1 << QUEUE_FLAG_NOWAIT))
> +#define QUEUE_FLAG_MQ_DEFAULT  ((1ULL << QUEUE_FLAG_IO_STAT) |         \
> +                                (1ULL << QUEUE_FLAG_SAME_COMP) |       \
> +                                (1ULL << QUEUE_FLAG_NOWAIT))
>
>  void blk_queue_flag_set(unsigned int flag, struct request_queue *q);
>  void blk_queue_flag_clear(unsigned int flag, struct request_queue *q);
> --
> 2.37.2
>

Tested-by: Joel Savitz <jsavitz@xxxxxxxxxx>
Reviewed-by: Joel Savitz <jsavitz@xxxxxxxxxx>




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux