Re: [v3 for-next 1/3] block: Reject unknown status tags in error injection rules

[Christoph Hellwig]

On Tue, Sep 29, 2026 at 12:16:32AM +0200, Md Haris Iqbal wrote:
> tag_to_blk_status() returns BLK_STS_OK both for the "OK" tag and for a
> tag it does not recognize, so a caller cannot tell the two apart. The
> block error injection code (which is the sole user of this function
> currently) relies on error_inject_add() later to reject adding a rule with
> status=BLK_STS_OK. This is okay for the current state of error injection.
> 
> The delay option added in the next commit makes the rule with status=OK
> valid, meaning the function tag_to_blk_status() now needs to explicitly
> match BLK_STS_OK for it, and fail for a tag it does not recognize. Hence
> make tag_to_blk_status() take a second param to update the matched status,
> and return true in case of a successful match. If a match is not found,
> the function tag_to_blk_status() returns false and *status remains
> unchanged.
> 
> A repeated status= where an invalid tag comes first is now rejected
> instead of being overridden by the later one.
> 
> Cc: Christoph Hellwig <hch@xxxxxx>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Md Haris Iqbal <haris.iqbal@xxxxxxxxx>
> ---
>  block/blk-core.c        | 14 ++++++--------
>  block/blk.h             |  2 +-
>  block/error-injection.c |  7 ++++---
>  3 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 13dc70e8f55d..c45846b9c50d 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -225,21 +225,19 @@ const char *blk_status_to_tag(blk_status_t status)
>  	return blk_errors[idx].tag;
>  }
>  
> -blk_status_t tag_to_blk_status(const char *tag)
> +bool tag_to_blk_status(const char *tag, blk_status_t *status)
>  {
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(blk_errors); i++) {
>  		if (blk_errors[i].tag &&
> -		    !strcmp(blk_errors[i].tag, tag))
> -			return (__force blk_status_t)i;
> +		    !strcmp(blk_errors[i].tag, tag)) {
> +			*status = (__force blk_status_t)i;
> +			return true;
> +		}
>  	}
>  
> -	/*
> -	 * Return BLK_STS_OK for mismatches as this function is intended to
> -	 * parse error status values.
> -	 */
> -	return BLK_STS_OK;
> +	return false;
>  }
>  
>  /**
> diff --git a/block/blk.h b/block/blk.h
> index 2cc03aa54c53..5fa54162c686 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -52,7 +52,7 @@ void blk_free_flush_queue(struct blk_flush_queue *q);
>  
>  const char *blk_status_to_str(blk_status_t status);
>  const char *blk_status_to_tag(blk_status_t status);
> -blk_status_t tag_to_blk_status(const char *tag);
> +bool tag_to_blk_status(const char *tag, blk_status_t *status);
>  enum req_op str_to_blk_op(const char *op);
>  
>  bool __blk_mq_unfreeze_queue(struct request_queue *q, bool force_atomic);
> diff --git a/block/error-injection.c b/block/error-injection.c
> index e14bc4b723ef..db543fe27630 100644
> --- a/block/error-injection.c
> +++ b/block/error-injection.c
> @@ -171,15 +171,16 @@ static int match_op(substring_t *args, enum req_op *op)
>  static int match_status(substring_t *args, blk_status_t *status)
>  {
>  	const char *tag;
> +	bool found;
>  
>  	tag = match_strdup(args);
>  	if (!tag)
>  		return -ENOMEM;
> -	*status = tag_to_blk_status(tag);
> -	if (!*status)
> +	found = tag_to_blk_status(tag, status);
> +	if (!found)
>  		pr_warn("invalid status '%s'\n", tag);
>  	kfree(tag);
> -	return 0;
> +	return found ? 0 : -EINVAL;

	if (!found) {
  		pr_warn("invalid status '%s'\n", tag);
		return -EINVAL;
	}
	return 0;

to keep the code a bit more readable.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@xxxxxx>