Re: crypto: skcipher - Unmap pages after an external error

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

 



On Tue, Sep 03, 2019 at 04:54:38PM +1000, Herbert Xu wrote:
>  int skcipher_walk_done(struct skcipher_walk *walk, int err)
>  {
> -	unsigned int n; /* bytes processed */
> -	bool more;
> -
> -	if (unlikely(err < 0))
> -		goto finish;
> +	unsigned int n = walk->nbytes - err;
> +	unsigned int nbytes;
>  
> -	n = walk->nbytes - err;
> -	walk->total -= n;
> -	more = (walk->total != 0);
> +	nbytes = walk->total - n;
>  
> -	if (likely(!(walk->flags & (SKCIPHER_WALK_PHYS |
> -				    SKCIPHER_WALK_SLOW |
> -				    SKCIPHER_WALK_COPY |
> -				    SKCIPHER_WALK_DIFF)))) {
> +	if (unlikely(err < 0)) {
> +		nbytes = 0;
> +		n = 0;
> +	} else if (likely(!(walk->flags & (SKCIPHER_WALK_PHYS |
> +					   SKCIPHER_WALK_SLOW |
> +					   SKCIPHER_WALK_COPY |
> +					   SKCIPHER_WALK_DIFF)))) {
>  unmap_src:
>  		skcipher_unmap_src(walk);
>  	} else if (walk->flags & SKCIPHER_WALK_DIFF) {
> @@ -134,25 +134,34 @@ int skcipher_walk_done(struct skcipher_walk *walk, int err)
>  			 * the algorithm requires it.
>  			 */
>  			err = -EINVAL;
> -			goto finish;
> -		}
> -		skcipher_done_slow(walk, n);
> -		goto already_advanced;
> +			nbytes = 0;
> +		} else
> +			n = skcipher_done_slow(walk, n);
>  	}
>  
> +	if (err > 0)
> +		err = 0;
> +
> +	walk->total = nbytes;
> +	walk->nbytes = nbytes;
> +
>  	scatterwalk_advance(&walk->in, n);
>  	scatterwalk_advance(&walk->out, n);
> -already_advanced:
> -	scatterwalk_done(&walk->in, 0, more);
> -	scatterwalk_done(&walk->out, 1, more);
> +	scatterwalk_done(&walk->in, 0, nbytes);
> +	scatterwalk_done(&walk->out, 1, nbytes);
>  
> -	if (more) {
> +	if (nbytes) {
>  		crypto_yield(walk->flags & SKCIPHER_WALK_SLEEP ?
>  			     CRYPTO_TFM_REQ_MAY_SLEEP : 0);
>  		return skcipher_walk_next(walk);
>  	}
> -	err = 0;
> -finish:
> +
> +	return skcipher_walk_unwind(walk, err);
> +}
> +EXPORT_SYMBOL_GPL(skcipher_walk_done);

Doesn't this re-introduce the same bug that my patch fixed -- that
scatterwalk_done() could be called after 0 bytes processed, causing a crash in
scatterwalk_pagedone()?

- Eric



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux