On Wed, Feb 14, 2018 at 02:38:37PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>
> Move all command header processing into separate function
> and perform those checks before acquiring SRCU read lock.
>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> drivers/infiniband/core/uverbs_main.c | 62 ++++++++++++++++++-----------------
> 1 file changed, 32 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> index 90e0b16aed1a..50d43c300112 100644
> +++ b/drivers/infiniband/core/uverbs_main.c
> @@ -660,6 +660,29 @@ static bool verify_command_idx(__u32 command, bool extended)
> uverbs_cmd_table[command];
> }
>
> +static ssize_t process_hdr(struct ib_uverbs_cmd_hdr *hdr,
> + __u32 *command, bool *extended)
> +{
> + __u32 flags;
> +
> + if (hdr->command & ~(__u32)(IB_USER_VERBS_CMD_FLAGS_MASK |
> + IB_USER_VERBS_CMD_COMMAND_MASK))
> + return -EINVAL;
> +
> + *command = hdr->command & IB_USER_VERBS_CMD_COMMAND_MASK;
> + flags = (hdr->command &
> + IB_USER_VERBS_CMD_FLAGS_MASK) >> IB_USER_VERBS_CMD_FLAGS_SHIFT;
> +
> + *extended = flags & IB_USER_VERBS_CMD_FLAG_EXTENDED;
> + if (flags & ~IB_USER_VERBS_CMD_FLAG_EXTENDED)
> + return -EINVAL;
er..
We don't need both
if (hdr->command & ~(__u32)(IB_USER_VERBS_CMD_FLAGS_MASK |
IB_USER_VERBS_CMD_COMMAND_MASK))
and
if (flags & ~IB_USER_VERBS_CMD_FLAG_EXTENDED)
return -EINVAL;
They test the same condition.
So my earlier comment about needing a stronger flags check is bogus -
we already have the stronger check. The original code was just
checking it twice, so it doesn't need this extra if.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
