* crypto: algif_skcipher - Fixed overflow when sndbuf is page aligned
@ 2010-11-30 8:51 Herbert Xu
2010-11-30 9:28 ` Dmitry Kasatkin
0 siblings, 1 reply; 3+ messages in thread
From: Herbert Xu @ 2010-11-30 8:51 UTC (permalink / raw)
To: Linux Crypto Mailing List
Hi:
Just noticed that algif_skcipher fails to apply the sk_sndbuf limit
correctly unless it is a multiple of PAGE_SIZE. What happens is
that the merge path will exceed sk_sndbuf causing the subsequent
limit comparison to fail as it tries to do an unsigned comparison
with a negative value.
This patch fixes the problem.
commit 0f6bb83cb12e4617e696ffa566f3fc6c092686e2
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue Nov 30 16:49:02 2010 +0800
crypto: algif_skcipher - Fixed overflow when sndbuf is page aligned
When sk_sndbuf is not a multiple of PAGE_SIZE, the limit tests
in sendmsg fail as the limit variable becomes negative and we're
using an unsigned comparison.
The same thing can happen if sk_sndbuf is lowered after a sendmsg
call.
This patch fixes this by always taking the signed maximum of limit
and 0 before we perform the comparison.
It also rounds the value of sk_sndbuf down to a multiple of PAGE_SIZE
so that we don't end up allocating a page only to use a small number
of bytes in it because we're bound by sk_sndbuf.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 9b2f440..1f33480 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -52,12 +52,18 @@ struct skcipher_ctx {
#define MAX_SGL_ENTS ((PAGE_SIZE - sizeof(struct skcipher_sg_list)) / \
sizeof(struct scatterlist) - 1)
-static inline bool skcipher_writable(struct sock *sk)
+static inline int skcipher_sndbuf(struct sock *sk)
{
struct alg_sock *ask = alg_sk(sk);
struct skcipher_ctx *ctx = ask->private;
- return ctx->used + PAGE_SIZE <= max_t(int, sk->sk_sndbuf, PAGE_SIZE);
+ return max_t(int, max_t(int, sk->sk_sndbuf & PAGE_MASK, PAGE_SIZE) -
+ ctx->used, 0);
+}
+
+static inline bool skcipher_writable(struct sock *sk)
+{
+ return PAGE_SIZE <= skcipher_sndbuf(sk);
}
static int skcipher_alloc_sgl(struct sock *sk)
@@ -245,7 +251,6 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
struct af_alg_control con = {};
long copied = 0;
bool enc = 0;
- int limit;
int err;
int i;
@@ -281,9 +286,6 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
memcpy(ctx->iv, con.iv->iv, ivsize);
}
- limit = max_t(int, sk->sk_sndbuf, PAGE_SIZE);
- limit -= ctx->used;
-
while (size) {
struct scatterlist *sg;
unsigned long len = size;
@@ -309,20 +311,16 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
ctx->used += len;
copied += len;
size -= len;
- limit -= len;
continue;
}
- if (limit < PAGE_SIZE) {
+ if (!skcipher_writable(sk)) {
err = skcipher_wait_for_wmem(sk, msg->msg_flags);
if (err)
goto unlock;
-
- limit = max_t(int, sk->sk_sndbuf, PAGE_SIZE);
- limit -= ctx->used;
}
- len = min_t(unsigned long, len, limit);
+ len = min_t(unsigned long, len, skcipher_sndbuf(sk));
err = skcipher_alloc_sgl(sk);
if (err)
@@ -352,7 +350,6 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
ctx->used += plen;
copied += plen;
size -= plen;
- limit -= plen;
sgl->cur++;
} while (len && sgl->cur < MAX_SGL_ENTS);
@@ -380,7 +377,6 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
struct skcipher_ctx *ctx = ask->private;
struct skcipher_sg_list *sgl;
int err = -EINVAL;
- int limit;
lock_sock(sk);
if (!ctx->more && ctx->used)
@@ -389,16 +385,10 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
if (!size)
goto done;
- limit = max_t(int, sk->sk_sndbuf, PAGE_SIZE);
- limit -= ctx->used;
-
- if (limit < PAGE_SIZE) {
+ if (!skcipher_writable(sk)) {
err = skcipher_wait_for_wmem(sk, flags);
if (err)
goto unlock;
-
- limit = max_t(int, sk->sk_sndbuf, PAGE_SIZE);
- limit -= ctx->used;
}
err = skcipher_alloc_sgl(sk);
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: crypto: algif_skcipher - Fixed overflow when sndbuf is page aligned
2010-11-30 8:51 crypto: algif_skcipher - Fixed overflow when sndbuf is page aligned Herbert Xu
@ 2010-11-30 9:28 ` Dmitry Kasatkin
2010-11-30 9:45 ` Herbert Xu
0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Kasatkin @ 2010-11-30 9:28 UTC (permalink / raw)
To: ext Herbert Xu; +Cc: Linux Crypto Mailing List
Hi,
What is the repo with algif patches?
Would like to try it..
- Dmitry
On 30/11/10 10:51, ext Herbert Xu wrote:
> Hi:
>
> Just noticed that algif_skcipher fails to apply the sk_sndbuf limit
> correctly unless it is a multiple of PAGE_SIZE. What happens is
> that the merge path will exceed sk_sndbuf causing the subsequent
> limit comparison to fail as it tries to do an unsigned comparison
> with a negative value.
>
> This patch fixes the problem.
>
> commit 0f6bb83cb12e4617e696ffa566f3fc6c092686e2
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Tue Nov 30 16:49:02 2010 +0800
>
> crypto: algif_skcipher - Fixed overflow when sndbuf is page aligned
>
> When sk_sndbuf is not a multiple of PAGE_SIZE, the limit tests
> in sendmsg fail as the limit variable becomes negative and we're
> using an unsigned comparison.
>
> The same thing can happen if sk_sndbuf is lowered after a sendmsg
> call.
>
> This patch fixes this by always taking the signed maximum of limit
> and 0 before we perform the comparison.
>
> It also rounds the value of sk_sndbuf down to a multiple of PAGE_SIZE
> so that we don't end up allocating a page only to use a small number
> of bytes in it because we're bound by sk_sndbuf.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index 9b2f440..1f33480 100644
> --- a/crypto/algif_skcipher.c
> +++ b/crypto/algif_skcipher.c
> @@ -52,12 +52,18 @@ struct skcipher_ctx {
> #define MAX_SGL_ENTS ((PAGE_SIZE - sizeof(struct skcipher_sg_list)) / \
> sizeof(struct scatterlist) - 1)
>
> -static inline bool skcipher_writable(struct sock *sk)
> +static inline int skcipher_sndbuf(struct sock *sk)
> {
> struct alg_sock *ask = alg_sk(sk);
> struct skcipher_ctx *ctx = ask->private;
>
> - return ctx->used + PAGE_SIZE <= max_t(int, sk->sk_sndbuf, PAGE_SIZE);
> + return max_t(int, max_t(int, sk->sk_sndbuf & PAGE_MASK, PAGE_SIZE) -
> + ctx->used, 0);
> +}
> +
> +static inline bool skcipher_writable(struct sock *sk)
> +{
> + return PAGE_SIZE <= skcipher_sndbuf(sk);
> }
>
> static int skcipher_alloc_sgl(struct sock *sk)
> @@ -245,7 +251,6 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
> struct af_alg_control con = {};
> long copied = 0;
> bool enc = 0;
> - int limit;
> int err;
> int i;
>
> @@ -281,9 +286,6 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
> memcpy(ctx->iv, con.iv->iv, ivsize);
> }
>
> - limit = max_t(int, sk->sk_sndbuf, PAGE_SIZE);
> - limit -= ctx->used;
> -
> while (size) {
> struct scatterlist *sg;
> unsigned long len = size;
> @@ -309,20 +311,16 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
> ctx->used += len;
> copied += len;
> size -= len;
> - limit -= len;
> continue;
> }
>
> - if (limit < PAGE_SIZE) {
> + if (!skcipher_writable(sk)) {
> err = skcipher_wait_for_wmem(sk, msg->msg_flags);
> if (err)
> goto unlock;
> -
> - limit = max_t(int, sk->sk_sndbuf, PAGE_SIZE);
> - limit -= ctx->used;
> }
>
> - len = min_t(unsigned long, len, limit);
> + len = min_t(unsigned long, len, skcipher_sndbuf(sk));
>
> err = skcipher_alloc_sgl(sk);
> if (err)
> @@ -352,7 +350,6 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
> ctx->used += plen;
> copied += plen;
> size -= plen;
> - limit -= plen;
> sgl->cur++;
> } while (len && sgl->cur < MAX_SGL_ENTS);
>
> @@ -380,7 +377,6 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
> struct skcipher_ctx *ctx = ask->private;
> struct skcipher_sg_list *sgl;
> int err = -EINVAL;
> - int limit;
>
> lock_sock(sk);
> if (!ctx->more && ctx->used)
> @@ -389,16 +385,10 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
> if (!size)
> goto done;
>
> - limit = max_t(int, sk->sk_sndbuf, PAGE_SIZE);
> - limit -= ctx->used;
> -
> - if (limit < PAGE_SIZE) {
> + if (!skcipher_writable(sk)) {
> err = skcipher_wait_for_wmem(sk, flags);
> if (err)
> goto unlock;
> -
> - limit = max_t(int, sk->sk_sndbuf, PAGE_SIZE);
> - limit -= ctx->used;
> }
>
> err = skcipher_alloc_sgl(sk);
>
> Cheers,
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: crypto: algif_skcipher - Fixed overflow when sndbuf is page aligned
2010-11-30 9:28 ` Dmitry Kasatkin
@ 2010-11-30 9:45 ` Herbert Xu
0 siblings, 0 replies; 3+ messages in thread
From: Herbert Xu @ 2010-11-30 9:45 UTC (permalink / raw)
To: Dmitry Kasatkin; +Cc: herbert, linux-crypto
Dmitry Kasatkin <dmitry.kasatkin@nokia.com> wrote:
> Hi,
>
> What is the repo with algif patches?
http://git.kernel.org/?p=linux/kernel/git/herbert/cryptodev-2.6.git;a=summary
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-11-30 9:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-30 8:51 crypto: algif_skcipher - Fixed overflow when sndbuf is page aligned Herbert Xu
2010-11-30 9:28 ` Dmitry Kasatkin
2010-11-30 9:45 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox