From: Stephan Mueller <hidden> Date: 2014-11-12 07:09:55
Hi,
This patch set adds AEAD and RNG support to the AF_ALG interface
exported by the kernel crypto API. By extending AF_ALG with AEAD and RNG
support, all cipher types the kernel crypto API allows access to are
now accessible from userspace.
The RNG support is stand-alone.
The AEAD implementation is added to algif_skcipher.c to prevent
re-implementation of the memory moving logic.
The extension for the AEAD support can be summarized with the following
types of changes:
* select the correct crypto API functions (either the ablkcipher
or the aead functions)
* apply the additional data needed for AEAD at the right time
(associated data, authentication tag) -- this includes the addition
of user space interfaces to allow setting this data.
* add the calculation for the memory size needed for encryption and
decryption.
In addition, the patch set adds a getsockopt implementation to skcipher to
allow user space to inquire about properties of the ciphers (IV size,
block size, authentication data size). This extension would be needed for a
generic user space usage of these ciphers.
The new AEAD and RNG interfaces are fully tested with the test application
provided at [1]. That test application exercises all newly added user space
interfaces.
The patch set was tested on x86_64 and i386.
[1] http://www.chronox.de/libkcapi.html
Stephan Mueller (8):
crypto: AF_ALG: add user space interface for AEAD
crypto: AF_ALG: user space interface for cipher info
crypto: AF_ALG: extend data structuers for AEAD
crypto: AF_ALG: crypto API calls to inline functions
crypto: AF_ALG: add AEAD support
crypto: AF_ALG: make setkey optional
crypto: AF_ALG: add random number generator support
crypto: AF_ALG: enable RNG interface compilation
crypto/Kconfig | 9 ++
crypto/Makefile | 1 +
crypto/af_alg.c | 20 +++
crypto/algif_rng.c | 186 +++++++++++++++++++++++
crypto/algif_skcipher.c | 350 ++++++++++++++++++++++++++++++++++++++++----
include/crypto/if_alg.h | 2 +
include/uapi/linux/if_alg.h | 10 ++
7 files changed, 550 insertions(+), 28 deletions(-)
create mode 100644 crypto/algif_rng.c
--
2.1.0
From: Stephan Mueller <hidden> Date: 2014-11-12 07:09:24
Enable compilation of the RNG AF_ALG support and provide a Kconfig
option to compile the RNG AF_ALG support.
Signed-off-by: Stephan Mueller <redacted>
---
crypto/Kconfig | 9 +++++++++
crypto/Makefile | 1 +
2 files changed, 10 insertions(+)
@@ -1505,6 +1505,15 @@ config CRYPTO_USER_API_SKCIPHERThisoptionenablestheuser-spacesinterfaceforsymmetrickeycipheralgorithms.+configCRYPTO_USER_API_RNG+tristate"User-space interface for random number generator algorithms"+depends onNET+selectCRYPTO_RNG+selectCRYPTO_USER_API+help+Thisoptionenablestheuser-spacesinterfaceforrandom+numbergeneratoralgorithms.+configCRYPTO_HASH_INFObool
@@ -99,6 +99,7 @@ obj-$(CONFIG_CRYPTO_GHASH) += ghash-generic.oobj-$(CONFIG_CRYPTO_USER_API)+=af_alg.oobj-$(CONFIG_CRYPTO_USER_API_HASH)+=algif_hash.oobj-$(CONFIG_CRYPTO_USER_API_SKCIPHER)+=algif_skcipher.o+obj-$(CONFIG_CRYPTO_USER_API_RNG)+=algif_rng.o## generic algorithms and the async_tx api
From: Stephan Mueller <hidden> Date: 2014-11-12 07:09:29
The current AF_ALG implementation requires that a userspace interface
implementation must provide a callback for setkey. Such a call is not
appliable to random number generators.
To prepare AF_ALG for the addition of a random number generator user
space interface, this function callback invocation is made optional.
Signed-off-by: Stephan Mueller <redacted>
---
crypto/af_alg.c | 3 +++
1 file changed, 3 insertions(+)
From: Stephan Mueller <hidden> Date: 2014-11-12 07:09:32
This patch adds the AEAD support for AF_ALG.
The AEAD implementation uses the entire memory handling and
infrastructure of the existing skcipher implementation.
To use AEAD, the user space consumer has to use the salg_type named
"aead". The AEAD extension only uses the bind callback as the key
differentiator. The previously added functions that select whether to
use AEAD or ablkcipher crypto API functions depend on the TFM type
allocated during the bind() call.
The addition of AEAD brings a bit of overhead to calculate the size of
the ciphertext, because the AEAD implementation of the kernel crypto API
makes implied assumption on the location of the authentication tag. When
performing an encryption, the tag will be added to the created
ciphertext (note, the tag is placed adjacent to the ciphertext). For
decryption, the caller must hand in the ciphertext with the tag appended
to the ciphertext. Therefore, the selection of the used memory
needs to add/subtract the tag size from the source/destination buffers
depending on the encryption type. The code is provided with comments
explainint when and how that operation is performed.
Note: The AF_ALG interface does not support zero length plaintext or
zero length ciphertext. Such zero length input data may be used if one
wants to access the hash implementation of an AEAD directly (e.g. the
GHASH of GCM or CMAC for CCM). However, this is a use case that is not
of interest. GHASH or CMAC is directly available via the hash AF_ALG
interface and we therefore do not need to take precautions for this
use case.
A fully working example using all aspects of AEAD is provided at
http://www.chronox.de/libkcapi.html
Signed-off-by: Stephan Mueller <redacted>
---
crypto/algif_skcipher.c | 153 +++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 144 insertions(+), 9 deletions(-)
@@ -399,6 +410,25 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,ctx->enc=enc;if(con.iv)memcpy(ctx->iv,con.iv->iv,ivsize);+/* AEAD authentication data handling */+if(ctx->aead){+if(con.aead_authsize)+err=crypto_aead_setauthsize(+crypto_aead_reqtfm(&ctx->u.aead_req),+con.aead_authsize);+if(err)+gotounlock;+/* set associated data */+memcpy(ctx->aead_assoc,+con.aead_assoc->aead_assoc,+con.aead_assoc->aead_assoclen);+sg_init_one(&ctx->sg_aead_assoc,+ctx->aead_assoc,+con.aead_assoc->aead_assoclen);+aead_request_set_assoc(&ctx->u.aead_req,+&ctx->sg_aead_assoc,+con.aead_assoc->aead_assoclen);+}}while(size){
@@ -547,10 +577,41 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,interr=-EAGAIN;intused;longcopied=0;+unsignedintaead_authsize_enc=0;+unsignedintaead_authsize_dec=0;lock_sock(sk);+/*+*AEADmemorystructure:Forencryption,thetagisappendedtothe+*ciphertextwhichimpliesthatthememoryallocatedfortheciphertext+*mustbeincreasedbythetaglength.Fordecryption,thetag+*isexpectedtobeconcatenatedtotheciphertext.Theplaintext+*thereforehasamemorysizeoftheciphertextminusthetaglength.+*+*Note:thismemorycalculationonlyworksbecausewerequirethe+*userspacecallerto:+**performencryptionbyinvokingtherecvfunctionwithabuffer+*lengthofciphertext+tagsize--thesendfunctioncanbe+*invokednormallywithjusttheplaintext.+**performadecryptionbyinvokingthethewritefunctionwith+*abufferholdingtheciphertext+tag(andsettingthe+*buffersizeaccordingly)--therecvfunctioncanbeinvoked+*normallywithjustthespaceneededfortheciphertext.+*Though,thecallershouldcheckforEBADMSGtocatchintegiry+*violations.+*/+if(ctx->aead){+if(ctx->enc)+aead_authsize_enc=crypto_aead_authsize(+crypto_aead_reqtfm(&ctx->u.aead_req));+else+aead_authsize_dec=crypto_aead_authsize(+crypto_aead_reqtfm(&ctx->u.aead_req));+}+for(iov=msg->msg_iov,iovlen=msg->msg_iovlen;iovlen>0;iovlen--,iov++){+/* size of the output data memory */unsignedlongseglen=iov->iov_len;char__user*from=iov->iov_base;
@@ -562,6 +623,7 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,while(!sg->length)sg++;+/* size of the input data memory */used=ctx->used;if(!used){err=skcipher_wait_for_data(sk,flags);
From: Stephan Mueller <hidden> Date: 2014-11-12 07:09:37
To avoid excessive branches and cluttering the code, all kernel crypto
API calls are extracted into separate inline functions. These functions
invoke either the ablkcipher or the aead crypto API function calls, as
necessary.
Signed-off-by: Stephan Mueller <redacted>
---
crypto/algif_skcipher.c | 141 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 124 insertions(+), 17 deletions(-)
From: Stephan Mueller <hidden> Date: 2014-11-12 07:09:41
The data structure holding the state of an ongoing symmetric cipher
operation is extended by the data variables needed for AEAD.
The request data structures are encapsulated by a union as the symmetric
cipher implementation is either exclusively used for "normal" symmetric
ciphers or for AEAD ciphers.
The define MAX_AEAD_ASSOCLEN restricts the size of the associated
authentication data. The kernel must allocate memory for this data to be
stored for the cipher operation. To prevent an excessive use of memory,
it is limited to 128 bytes, which is considered to be a sensible size.
Signed-off-by: Stephan Mueller <redacted>
---
crypto/algif_skcipher.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
From: Stephan Mueller <hidden> Date: 2014-11-12 07:09:50
AEAD requires the following data in addition to normal symmetric
ciphers:
* Associated authentication data of arbitrary length
* Authentication tag for decryption
* Length of authentication tag for encryption
The authentication tag data is communicated as part of the actual
ciphertext as mandated by the kernel crypto API. Therefore we only need
to provide a user space interface for the associated authentication data
as well as for the authentication tag length.
This patch adds both as a setsockopt interface that is identical to the
AF_ALG interface for setting an IV and for selecting the cipher
operation type (encrypt or decrypt).
Signed-off-by: Stephan Mueller <redacted>
---
crypto/af_alg.c | 17 +++++++++++++++++
include/crypto/if_alg.h | 2 ++
include/uapi/linux/if_alg.h | 7 +++++++
3 files changed, 26 insertions(+)
From: Stephan Mueller <hidden> Date: 2014-11-12 07:10:39
The AF_ALG interface allows normal cipher (hash, encrypt, decrypt).
However, it does not allow user space to obtain the following generic
information about the currently active cipher:
* block size of the cipher
* IV size of the cipher
* for AEAD, the maximum authentication tag size
The patch adds a getsockopt interface for the symmetric ciphers to
answer such information requests from user space.
The kernel crypto API function calls are used to obtain the real data.
As all data are simple integer values, the getsockopt handler function
uses put_user() to return the integer value to user space in the
*optval parameter of getsockopt.
Signed-off-by: Stephan Mueller <redacted>
---
crypto/algif_skcipher.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-
include/uapi/linux/if_alg.h | 3 +++
2 files changed, 48 insertions(+), 1 deletion(-)
From: Stephan Mueller <hidden> Date: 2014-11-12 07:11:32
This patch adds the random number generator support for AF_ALG.
A random number generator's purpose is to generate data without
requiring the caller to provide any data. Therefore, the AF_ALG
interface handler for RNGs only implements a callback handler for
recvmsg.
The following parameters provided with a recvmsg are processed by the
RNG callback handler:
* sock - to resolve the RNG context data structure accessing the
RNG instance private to the socket
* len - this parameter allows userspace callers to specify how
many random bytes the RNG shall produce and return. As the
kernel context for the RNG allocates a buffer of 128 bytes to
store random numbers before copying them to userspace, the len
parameter is checked that it is not larger than 128. If a
caller wants more random numbers, a new request for recvmsg
shall be made.
The size of 128 bytes is chose because of the following considerations:
* to increase the memory footprint of the kernel too much (note,
that would be 128 bytes per open socket)
* 128 is divisible by any typical cryptographic block size an
RNG may have
* A request for random numbers typically only shall supply small
amount of data like for keys or IVs that should only require
one invocation of the recvmsg function.
Note, during instantiation of the RNG, the code checks whether the RNG
implementation requires seeding. If so, the RNG is seeded with output
from get_random_bytes.
A fully working example using all aspects of the RNG interface is
provided at http://www.chronox.de/libkcapi.html
Signed-off-by: Stephan Mueller <redacted>
---
crypto/algif_rng.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 186 insertions(+)
create mode 100644 crypto/algif_rng.c
@@ -0,0 +1,186 @@+/*+*algif_rng:User-spaceinterfaceforrandomnumbergenerators+*+*Thisfileprovidestheuser-spaceAPIforrandomnumbergenerators.+*+*Copyright(C)2014,StephanMueller<smueller@chronox.de>+*+*Redistributionanduseinsourceandbinaryforms,withorwithout+*modification,arepermittedprovidedthatthefollowingconditions+*aremet:+*1.Redistributionsofsourcecodemustretaintheabovecopyright+*notice,andtheentirepermissionnoticeinitsentirety,+*includingthedisclaimerofwarranties.+*2.Redistributionsinbinaryformmustreproducetheabovecopyright+*notice,thislistofconditionsandthefollowingdisclaimerinthe+*documentationand/orothermaterialsprovidedwiththedistribution.+*3.Thenameoftheauthormaynotbeusedtoendorseorpromote+*productsderivedfromthissoftwarewithoutspecificprior+*writtenpermission.+*+*ALTERNATIVELY,thisproductmaybedistributedunderthetermsof+*theGNUGeneralPublicLicense,inwhichcasetheprovisionsoftheGPL2are+*requiredINSTEADOFtheaboverestrictions.(Thisclauseis+*necessaryduetoapotentialbadinteractionbetweentheGPLand+*therestrictionscontainedinaBSD-stylecopyright.)+*+*THISSOFTWAREISPROVIDED``ASIS''ANDANYEXPRESSORIMPLIED+*WARRANTIES,INCLUDING,BUTNOTLIMITEDTO,THEIMPLIEDWARRANTIES+*OFMERCHANTABILITYANDFITNESSFORAPARTICULARPURPOSE,ALLOF+*WHICHAREHEREBYDISCLAIMED.INNOEVENTSHALLTHEAUTHORBE+*LIABLEFORANYDIRECT,INDIRECT,INCIDENTAL,SPECIAL,EXEMPLARY,OR+*CONSEQUENTIALDAMAGES(INCLUDING,BUTNOTLIMITEDTO,PROCUREMENT+*OFSUBSTITUTEGOODSORSERVICES;LOSSOFUSE,DATA,ORPROFITS;OR+*BUSINESSINTERRUPTION)HOWEVERCAUSEDANDONANYTHEORYOF+*LIABILITY,WHETHERINCONTRACT,STRICTLIABILITY,ORTORT+*(INCLUDINGNEGLIGENCEOROTHERWISE)ARISINGINANYWAYOUTOFTHE+*USEOFTHISSOFTWARE,EVENIFNOTADVISEDOFTHEPOSSIBILITYOFSUCH+*DAMAGE.+*/++#include<linux/module.h>+#include<crypto/rng.h>+#include<linux/random.h>+#include<crypto/if_alg.h>+#include<linux/net.h>+#include<net/sock.h>++MODULE_LICENSE("GPL");+MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");+MODULE_DESCRIPTION("User-space interface for random number generators");++structrng_ctx{+#define MAXSIZE 128+u8result[MAXSIZE];+unsignedintlen;+structcrypto_rng*drng;+};++staticintrng_recvmsg(structkiocb*unused,structsocket*sock,+structmsghdr*msg,size_tlen,intflags)+{+structsock*sk=sock->sk;+structalg_sock*ask=alg_sk(sk);+structrng_ctx*ctx=ask->private;+interr=-EFAULT;++if(0==len)+return0;+if(MAXSIZE<len)+len=MAXSIZE;++lock_sock(sk);+len=crypto_rng_get_bytes(ctx->drng,ctx->result,len);+if(0>len)+gotounlock;++err=memcpy_toiovec(msg->msg_iov,ctx->result,len);+memset(ctx->result,0,err);++unlock:+release_sock(sk);++returnerr?err:len;+}++staticstructproto_opsalgif_rng_ops={+.family=PF_ALG,++.connect=sock_no_connect,+.socketpair=sock_no_socketpair,+.getname=sock_no_getname,+.ioctl=sock_no_ioctl,+.listen=sock_no_listen,+.shutdown=sock_no_shutdown,+.getsockopt=sock_no_getsockopt,+.mmap=sock_no_mmap,+.bind=sock_no_bind,+.accept=sock_no_accept,+.setsockopt=sock_no_setsockopt,+.poll=sock_no_poll,+.sendmsg=sock_no_sendmsg,+.sendpage=sock_no_sendpage,++.release=af_alg_release,+.recvmsg=rng_recvmsg,+};++staticvoid*rng_bind(constchar*name,u32type,u32mask)+{+returncrypto_alloc_rng(name,type,mask);+}++staticvoidrng_release(void*private)+{+crypto_free_rng(private);+}++staticvoidrng_sock_destruct(structsock*sk)+{+structalg_sock*ask=alg_sk(sk);+structrng_ctx*ctx=ask->private;++memset(ctx->result,0,MAXSIZE);+sock_kfree_s(sk,ctx,ctx->len);+af_alg_release_parent(sk);+}++staticintrng_accept_parent(void*private,structsock*sk)+{+structrng_ctx*ctx;+structalg_sock*ask=alg_sk(sk);+unsignedintlen=sizeof(*ctx);+intseedsize=crypto_rng_seedsize(private);+intret=-ENOMEM;++ctx=sock_kmalloc(sk,len,GFP_KERNEL);+if(!ctx)+return-ENOMEM;+memset(ctx->result,0,MAXSIZE);++ctx->len=len;++if(seedsize){+u8*buf=kmalloc(seedsize,GFP_KERNEL);+if(!buf)+gotoerr;+get_random_bytes(buf,seedsize);+ret=crypto_rng_reset(private,buf,len);+kzfree(buf);+if(ret)+gotoerr;+}++ctx->drng=private;+ask->private=ctx;+sk->sk_destruct=rng_sock_destruct;++return0;++err:+sock_kfree_s(sk,ctx,len);+returnret;+}++staticconststructaf_alg_typealgif_type_rng={+.bind=rng_bind,+.release=rng_release,+.accept=rng_accept_parent,+.ops=&algif_rng_ops,+.name="rng",+.owner=THIS_MODULE+};++staticint__initrng_init(void)+{+returnaf_alg_register_type(&algif_type_rng);+}++void__exitrng_exit(void)+{+interr=af_alg_unregister_type(&algif_type_rng);+BUG_ON(err);+}++module_init(rng_init);+module_exit(rng_exit);
From: Daniel Borkmann <hidden> Date: 2014-11-12 16:16:08
On 11/12/2014 08:05 AM, Stephan Mueller wrote:
This patch adds the random number generator support for AF_ALG.
A random number generator's purpose is to generate data without
requiring the caller to provide any data. Therefore, the AF_ALG
interface handler for RNGs only implements a callback handler for
recvmsg.
This looks buggy.
If copy_to_user() fails from within memcpy_toiovec(), we call memset()
with a negative return value which is interpreted as size_t and thus
causes a buffer overflow writing beyond ctx->result, no?
If it succeeds, we call memset(ctx->result, 0, 0) .....
From: Stephan Mueller <hidden> Date: 2014-11-12 16:54:33
Am Mittwoch, 12. November 2014, 17:15:52 schrieb Daniel Borkmann:
Hi Daniel,
thanks for the comments.
On 11/12/2014 08:05 AM, Stephan Mueller wrote:
quoted
This patch adds the random number generator support for AF_ALG.
A random number generator's purpose is to generate data without
requiring the caller to provide any data. Therefore, the AF_ALG
interface handler for RNGs only implements a callback handler for
recvmsg.
if (len == 0)
...
[And also other places.]
We don't use Yoda condition style in the kernel.
Well, there is a very good reason for using the approach I have: we all have
done the error of forgetting the second = sign.
In my case, the compiler will complain and we fix the error right away.
In your case, nobody is complaining but we introduced a nasty, potentially
hard to debug error. Thus, I very much like to keep my version just to be on
the safe side.
Note, there was even a backdoor I have seen where the missing 2nd equal sign
introduced a privilege escalation.
Therefore, my standard coding practice is to have a fixed value on the left
side and the variable on the right side of any comparison.
This looks buggy.
If copy_to_user() fails from within memcpy_toiovec(), we call memset()
with a negative return value which is interpreted as size_t and thus
causes a buffer overflow writing beyond ctx->result, no?
If it succeeds, we call memset(ctx->result, 0, 0) .....
Right, good catch, I have to add a catch for negative error here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Daniel Borkmann <hidden> Date: 2014-11-12 17:23:39
On 11/12/2014 05:54 PM, Stephan Mueller wrote:
Am Mittwoch, 12. November 2014, 17:15:52 schrieb Daniel Borkmann:
quoted
On 11/12/2014 08:05 AM, Stephan Mueller wrote:
quoted
This patch adds the random number generator support for AF_ALG.
A random number generator's purpose is to generate data without
requiring the caller to provide any data. Therefore, the AF_ALG
interface handler for RNGs only implements a callback handler for
recvmsg.
if (len == 0)
...
[And also other places.]
We don't use Yoda condition style in the kernel.
Well, there is a very good reason for using the approach I have: we all have
done the error of forgetting the second = sign.
In my case, the compiler will complain and we fix the error right away.
In your case, nobody is complaining but we introduced a nasty, potentially
hard to debug error. Thus, I very much like to keep my version just to be on
the safe side.
Note, there was even a backdoor I have seen where the missing 2nd equal sign
introduced a privilege escalation.
Therefore, my standard coding practice is to have a fixed value on the left
side and the variable on the right side of any comparison.
I understand, but then please add this proposal first into ...
Documentation/CodingStyle
The problem is that while the rest of the kernel does not follow
this coding style, it's also much harder to read and/or program
this way for people not being used to. So the danger of bugs
slipping in this way is at least equally high. Besides that, this
argument would also only account for '==' checks.
This looks buggy.
If copy_to_user() fails from within memcpy_toiovec(), we call memset()
with a negative return value which is interpreted as size_t and thus
causes a buffer overflow writing beyond ctx->result, no?
If it succeeds, we call memset(ctx->result, 0, 0) .....
Right, good catch, I have to add a catch for negative error here.
Hm? Don't you rather mean to say to unconditionally do something like ...
memzero_explicit(ctx->result, len);
...
From: Stephan Mueller <hidden> Date: 2014-11-12 17:46:54
Am Mittwoch, 12. November 2014, 18:23:27 schrieb Daniel Borkmann:
Hi Daniel,
On 11/12/2014 05:54 PM, Stephan Mueller wrote:
quoted
Am Mittwoch, 12. November 2014, 17:15:52 schrieb Daniel Borkmann:
quoted
On 11/12/2014 08:05 AM, Stephan Mueller wrote:
quoted
This patch adds the random number generator support for AF_ALG.
A random number generator's purpose is to generate data without
requiring the caller to provide any data. Therefore, the AF_ALG
interface handler for RNGs only implements a callback handler for
recvmsg.
if (len == 0)
...
[And also other places.]
We don't use Yoda condition style in the kernel.
Well, there is a very good reason for using the approach I have: we
all have done the error of forgetting the second = sign.
In my case, the compiler will complain and we fix the error right
away.
In your case, nobody is complaining but we introduced a nasty,
potentially hard to debug error. Thus, I very much like to keep my
version just to be on the safe side.
Note, there was even a backdoor I have seen where the missing 2nd
equal sign introduced a privilege escalation.
Therefore, my standard coding practice is to have a fixed value on
the left side and the variable on the right side of any comparison.
I understand, but then please add this proposal first into ...
Documentation/CodingStyle
The problem is that while the rest of the kernel does not follow
this coding style, it's also much harder to read and/or program
this way for people not being used to. So the danger of bugs
slipping in this way is at least equally high. Besides that, this
argument would also only account for '==' checks.
This looks buggy.
If copy_to_user() fails from within memcpy_toiovec(), we call
memset()
with a negative return value which is interpreted as size_t and thus
causes a buffer overflow writing beyond ctx->result, no?
If it succeeds, we call memset(ctx->result, 0, 0) .....
Right, good catch, I have to add a catch for negative error here.
Hm? Don't you rather mean to say to unconditionally do something like
...
memzero_explicit(ctx->result, len);
Sorry, I was not clear:
* I need to catch a failing memcpy, but not return an error.
* I unconditionally use the memset after memcpy as you indicated. Once
the cryptodev tree contains the memzero_explicit call, I will start
picking up that function.
Essentially, I throught of the line you suggested.
Ciao
Stephan
From: Daniel Borkmann <hidden> Date: 2014-11-12 17:52:18
On 11/12/2014 06:46 PM, Stephan Mueller wrote:
...
* I unconditionally use the memset after memcpy as you indicated. Once
the cryptodev tree contains the memzero_explicit call, I will start
picking up that function.
Herbert merged it actually in this morning, so it's already part of
the cryptodev tree by now.
Essentially, I throught of the line you suggested.