* Re: [PATCH v2 1/3] crypto: brcm: DT documentation for Broadcom SPU driver
From: Rob Herring @ 2016-12-09 21:26 UTC (permalink / raw)
To: Rob Rice
Cc: Herbert Xu, David S. Miller, Mark Rutland,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ray Jui, Scott Branden,
Jon Mason, bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
Catalin Marinas, Will Deacon,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Steve Lin
In-Reply-To: <1480714499-1476-2-git-send-email-rob.rice-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
On Fri, Dec 02, 2016 at 04:34:57PM -0500, Rob Rice wrote:
> Device tree documentation for Broadcom Secure Processing Unit
> (SPU) crypto driver.
>
> Signed-off-by: Steve Lin <steven.lin1-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Rob Rice <rob.rice-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> ---
> .../devicetree/bindings/crypto/brcm,spu-crypto.txt | 25 ++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>
> diff --git a/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
> new file mode 100644
> index 0000000..e5fe942
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
> @@ -0,0 +1,25 @@
> +The Broadcom Secure Processing Unit (SPU) driver supports symmetric
Bindings describe h/w, not drivers.
> +cryptographic offload for Broadcom SoCs with SPU hardware. A SoC may have
> +multiple SPU hardware blocks.
> +
> +Required properties:
> +- compatible : Should be "brcm,spum-crypto" for devices with SPU-M hardware
Additionally, you should have SoC specific compatible here.
> + (e.g., Northstar2) or "brcm,spum-nsp-crypto" for the Northstar Plus variant
> + of the SPU-M hardware.
> +
> +- reg: Should contain SPU registers location and length.
> +- mboxes: A list of mailbox channels to be used by the kernel driver. Mailbox
> +channels correspond to DMA rings on the device.
What determines the mbox assignment?
Needs to specify how many.
> +
> +Example:
> + spu-crypto@612d0000 {
Just crypto@...
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 1/2] sparc: fix a building error reported by kbuild
From: Sam Ravnborg @ 2016-12-09 21:58 UTC (permalink / raw)
To: Gonglei
Cc: linux-kernel, qemu-devel, virtio-dev, virtualization,
linux-crypto, luonengjun, mst, stefanha, weidong.huang, wu.wubin,
xin.zeng, claudio.fontana, herbert, pasic, davem, jianjay.zhou,
hanweidong, arei.gonglei, cornelia.huck, xuquan8, longpeng2,
wanzongshun, sparclinux
In-Reply-To: <1481171829-116496-2-git-send-email-arei.gonglei@huawei.com>
Hi Gonglei.
On Thu, Dec 08, 2016 at 12:37:08PM +0800, Gonglei wrote:
> >> arch/sparc/include/asm/topology_64.h:44:44:
> error: implicit declaration of function 'cpu_data'
> [-Werror=implicit-function-declaration]
>
> #define topology_physical_package_id(cpu) (cpu_data(cpu).proc_id)
> ^
> Let's include cpudata.h in topology_64.h.
>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: sparclinux@vger.kernel.org
> Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
> arch/sparc/include/asm/topology_64.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/sparc/include/asm/topology_64.h b/arch/sparc/include/asm/topology_64.h
> index 7b4898a..2255430 100644
> --- a/arch/sparc/include/asm/topology_64.h
> +++ b/arch/sparc/include/asm/topology_64.h
> @@ -4,6 +4,7 @@
> #ifdef CONFIG_NUMA
>
> #include <asm/mmzone.h>
> +#include <asm/cpudata.h>
Nitpick - if you are going to resend this patch, then please
order the two includes in alphabetic order.
For two includes this looks like bikeshedding, but when we add
more having them in a defined arder prevents merge conflicts.
And makes it readable too.
We also sometimes order the includes with the longest lines topmost,
and lines with the ame length are ordered alphabetically.
But this is not seen so often.
Sam
^ permalink raw reply
* Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Eric Biggers @ 2016-12-09 23:08 UTC (permalink / raw)
To: linux-crypto
Cc: linux-kernel, linux-mm, kernel-hardening, Herbert Xu,
Andrew Lutomirski, Stephan Mueller
In the 4.9 kernel, virtually-mapped stacks will be supported and enabled by
default on x86_64. This has been exposing a number of problems in which
on-stack buffers are being passed into the crypto API, which to support crypto
accelerators operates on 'struct page' rather than on virtual memory.
Some of these problems have already been fixed, but I was wondering how many
problems remain, so I briefly looked through all the callers of sg_set_buf() and
sg_init_one(). Overall I found quite a few remaining problems, detailed below.
The following crypto drivers initialize a scatterlist to point into an
ahash_request, which may have been allocated on the stack with
AHASH_REQUEST_ON_STACK():
drivers/crypto/bfin_crc.c:351
drivers/crypto/qce/sha.c:299
drivers/crypto/sahara.c:973,988
drivers/crypto/talitos.c:1910
drivers/crypto/ccp/ccp-crypto-aes-cmac.c:105,119,142
drivers/crypto/ccp/ccp-crypto-sha.c:95,109,124
drivers/crypto/qce/sha.c:325
The following crypto drivers initialize a scatterlist to point into an
ablkcipher_request, which may have been allocated on the stack with
SKCIPHER_REQUEST_ON_STACK():
drivers/crypto/ccp/ccp-crypto-aes-xts.c:162
drivers/crypto/ccp/ccp-crypto-aes.c:94
And these other places do crypto operations on buffers clearly on the stack:
drivers/net/wireless/intersil/orinoco/mic.c:72
drivers/usb/wusbcore/crypto.c:264
net/ceph/crypto.c:182
net/rxrpc/rxkad.c:737,1000
security/keys/encrypted-keys/encrypted.c:500
fs/cifs/smbencrypt.c:96
Note: I almost certainly missed some, since I excluded places where the use of a
stack buffer was not obvious to me. I also excluded AEAD algorithms since there
isn't an AEAD_REQUEST_ON_STACK() macro (yet).
The "good" news with these bugs is that on x86_64 without CONFIG_DEBUG_SG=y or
CONFIG_DEBUG_VIRTUAL=y, you can still do virt_to_page() and then page_address()
on a vmalloc address and get back the same address, even though you aren't
*supposed* to be able to do this. This will make things still work for most
people. The bad news is that if you happen to have consumed just about 1 page
(or N pages) of your stack at the time you call the crypto API, your stack
buffer may actually span physically non-contiguous pages, so the crypto
algorithm will scribble over some unrelated page. Also, hardware crypto drivers
which actually do operate on physical memory will break too.
So I am wondering: is the best solution really to make all these crypto API
algorithms and users use heap buffers, as opposed to something like maintaining
a lowmem alias for the stack, or introducing a more general function to convert
buffers (possibly in the vmalloc space) into scatterlists? And if the current
solution is desired, who is going to fix all of these bugs and when?
Eric
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 7/7] hwrng: core: Remove two unused include
From: kbuild test robot @ 2016-12-10 1:27 UTC (permalink / raw)
To: Corentin Labbe
Cc: kbuild-all, mpm, herbert, arnd, gregkh, linux-crypto,
linux-kernel, Corentin Labbe
In-Reply-To: <1481293299-21697-7-git-send-email-clabbe.montjoie@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 23999 bytes --]
Hi Corentin,
[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v4.9-rc8 next-20161209]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Corentin-Labbe/hwrng-core-do-not-use-multiple-blank-lines/20161210-072632
config: i386-randconfig-x007-201649 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from include/linux/linkage.h:4:0,
from include/linux/kernel.h:6,
from include/linux/delay.h:10,
from drivers/char/hw_random/core.c:13:
drivers/char/hw_random/core.c: In function 'rng_dev_open':
>> drivers/char/hw_random/core.c:169:11: error: dereferencing pointer to incomplete type 'struct file'
if ((filp->f_mode & FMODE_READ) == 0)
^
include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/char/hw_random/core.c:169:2: note: in expansion of macro 'if'
if ((filp->f_mode & FMODE_READ) == 0)
^~
>> drivers/char/hw_random/core.c:169:22: error: 'FMODE_READ' undeclared (first use in this function)
if ((filp->f_mode & FMODE_READ) == 0)
^
include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/char/hw_random/core.c:169:2: note: in expansion of macro 'if'
if ((filp->f_mode & FMODE_READ) == 0)
^~
drivers/char/hw_random/core.c:169:22: note: each undeclared identifier is reported only once for each function it appears in
if ((filp->f_mode & FMODE_READ) == 0)
^
include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/char/hw_random/core.c:169:2: note: in expansion of macro 'if'
if ((filp->f_mode & FMODE_READ) == 0)
^~
>> drivers/char/hw_random/core.c:171:21: error: 'FMODE_WRITE' undeclared (first use in this function)
if (filp->f_mode & FMODE_WRITE)
^
include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
drivers/char/hw_random/core.c:171:2: note: in expansion of macro 'if'
if (filp->f_mode & FMODE_WRITE)
^~
drivers/char/hw_random/core.c: In function 'rng_dev_read':
>> drivers/char/hw_random/core.c:221:23: error: 'O_NONBLOCK' undeclared (first use in this function)
!(filp->f_flags & O_NONBLOCK));
^~~~~~~~~~
drivers/char/hw_random/core.c: At top level:
>> drivers/char/hw_random/core.c:272:21: error: variable 'rng_chrdev_ops' has initializer but incomplete type
static const struct file_operations rng_chrdev_ops = {
^~~~~~~~~~~~~~~
>> drivers/char/hw_random/core.c:273:2: error: unknown field 'owner' specified in initializer
.owner = THIS_MODULE,
^
In file included from include/linux/linkage.h:6:0,
from include/linux/kernel.h:6,
from include/linux/delay.h:10,
from drivers/char/hw_random/core.c:13:
include/linux/export.h:37:21: warning: excess elements in struct initializer
#define THIS_MODULE ((struct module *)0)
^
>> drivers/char/hw_random/core.c:273:12: note: in expansion of macro 'THIS_MODULE'
.owner = THIS_MODULE,
^~~~~~~~~~~
include/linux/export.h:37:21: note: (near initialization for 'rng_chrdev_ops')
#define THIS_MODULE ((struct module *)0)
^
>> drivers/char/hw_random/core.c:273:12: note: in expansion of macro 'THIS_MODULE'
.owner = THIS_MODULE,
^~~~~~~~~~~
>> drivers/char/hw_random/core.c:274:2: error: unknown field 'open' specified in initializer
.open = rng_dev_open,
^
>> drivers/char/hw_random/core.c:274:11: warning: excess elements in struct initializer
.open = rng_dev_open,
^~~~~~~~~~~~
drivers/char/hw_random/core.c:274:11: note: (near initialization for 'rng_chrdev_ops')
>> drivers/char/hw_random/core.c:275:2: error: unknown field 'read' specified in initializer
.read = rng_dev_read,
^
drivers/char/hw_random/core.c:275:11: warning: excess elements in struct initializer
.read = rng_dev_read,
^~~~~~~~~~~~
drivers/char/hw_random/core.c:275:11: note: (near initialization for 'rng_chrdev_ops')
>> drivers/char/hw_random/core.c:276:2: error: unknown field 'llseek' specified in initializer
.llseek = noop_llseek,
^
>> drivers/char/hw_random/core.c:276:13: error: 'noop_llseek' undeclared here (not in a function)
.llseek = noop_llseek,
^~~~~~~~~~~
drivers/char/hw_random/core.c:276:13: warning: excess elements in struct initializer
drivers/char/hw_random/core.c:276:13: note: (near initialization for 'rng_chrdev_ops')
>> drivers/char/hw_random/core.c:272:37: error: storage size of 'rng_chrdev_ops' isn't known
static const struct file_operations rng_chrdev_ops = {
^~~~~~~~~~~~~~
vim +169 drivers/char/hw_random/core.c
92c7987e7 Corentin Labbe 2016-12-09 7 * Please read Documentation/hw_random.txt for details on use.
92c7987e7 Corentin Labbe 2016-12-09 8 *
92c7987e7 Corentin Labbe 2016-12-09 9 * This software may be used and distributed according to the terms
92c7987e7 Corentin Labbe 2016-12-09 10 * of the GNU General Public License, incorporated herein by reference.
844dd05fe Michael Buesch 2006-06-26 11 */
844dd05fe Michael Buesch 2006-06-26 12
b70f09c75 Corentin Labbe 2016-12-09 @13 #include <linux/delay.h>
844dd05fe Michael Buesch 2006-06-26 14 #include <linux/device.h>
b70f09c75 Corentin Labbe 2016-12-09 15 #include <linux/err.h>
844dd05fe Michael Buesch 2006-06-26 16 #include <linux/hw_random.h>
844dd05fe Michael Buesch 2006-06-26 17 #include <linux/kernel.h>
be4000bc4 Torsten Duwe 2014-06-14 18 #include <linux/kthread.h>
b70f09c75 Corentin Labbe 2016-12-09 19 #include <linux/miscdevice.h>
b70f09c75 Corentin Labbe 2016-12-09 20 #include <linux/module.h>
d9e797261 Kees Cook 2014-03-03 21 #include <linux/random.h>
b70f09c75 Corentin Labbe 2016-12-09 22 #include <linux/slab.h>
b70f09c75 Corentin Labbe 2016-12-09 23 #include <linux/uaccess.h>
844dd05fe Michael Buesch 2006-06-26 24
844dd05fe Michael Buesch 2006-06-26 25 #define RNG_MODULE_NAME "hw_random"
844dd05fe Michael Buesch 2006-06-26 26
844dd05fe Michael Buesch 2006-06-26 27 static struct hwrng *current_rng;
be4000bc4 Torsten Duwe 2014-06-14 28 static struct task_struct *hwrng_fill;
844dd05fe Michael Buesch 2006-06-26 29 static LIST_HEAD(rng_list);
9372b35e1 Rusty Russell 2014-12-08 30 /* Protects rng_list and current_rng */
844dd05fe Michael Buesch 2006-06-26 31 static DEFINE_MUTEX(rng_mutex);
9372b35e1 Rusty Russell 2014-12-08 32 /* Protects rng read functions, data_avail, rng_buffer and rng_fillbuf */
9372b35e1 Rusty Russell 2014-12-08 33 static DEFINE_MUTEX(reading_mutex);
9996508b3 Ian Molton 2009-12-01 34 static int data_avail;
be4000bc4 Torsten Duwe 2014-06-14 35 static u8 *rng_buffer, *rng_fillbuf;
0f734e6e7 Torsten Duwe 2014-06-14 36 static unsigned short current_quality;
0f734e6e7 Torsten Duwe 2014-06-14 37 static unsigned short default_quality; /* = 0; default to "off" */
be4000bc4 Torsten Duwe 2014-06-14 38
be4000bc4 Torsten Duwe 2014-06-14 39 module_param(current_quality, ushort, 0644);
be4000bc4 Torsten Duwe 2014-06-14 40 MODULE_PARM_DESC(current_quality,
be4000bc4 Torsten Duwe 2014-06-14 41 "current hwrng entropy estimation per mill");
0f734e6e7 Torsten Duwe 2014-06-14 42 module_param(default_quality, ushort, 0644);
0f734e6e7 Torsten Duwe 2014-06-14 43 MODULE_PARM_DESC(default_quality,
0f734e6e7 Torsten Duwe 2014-06-14 44 "default entropy content of hwrng per mill");
be4000bc4 Torsten Duwe 2014-06-14 45
ff77c150f Herbert Xu 2014-12-23 46 static void drop_current_rng(void);
90ac41bd4 Herbert Xu 2014-12-23 47 static int hwrng_init(struct hwrng *rng);
be4000bc4 Torsten Duwe 2014-06-14 48 static void start_khwrngd(void);
f7f154f12 Rusty Russell 2013-03-05 49
d3cc79964 Amit Shah 2014-07-10 50 static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
d3cc79964 Amit Shah 2014-07-10 51 int wait);
d3cc79964 Amit Shah 2014-07-10 52
f7f154f12 Rusty Russell 2013-03-05 53 static size_t rng_buffer_size(void)
f7f154f12 Rusty Russell 2013-03-05 54 {
f7f154f12 Rusty Russell 2013-03-05 55 return SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES;
f7f154f12 Rusty Russell 2013-03-05 56 }
844dd05fe Michael Buesch 2006-06-26 57
d3cc79964 Amit Shah 2014-07-10 58 static void add_early_randomness(struct hwrng *rng)
d3cc79964 Amit Shah 2014-07-10 59 {
d3cc79964 Amit Shah 2014-07-10 60 int bytes_read;
6d4952d9d Andrew Lutomirski 2016-10-17 61 size_t size = min_t(size_t, 16, rng_buffer_size());
d3cc79964 Amit Shah 2014-07-10 62
9372b35e1 Rusty Russell 2014-12-08 63 mutex_lock(&reading_mutex);
6d4952d9d Andrew Lutomirski 2016-10-17 64 bytes_read = rng_get_data(rng, rng_buffer, size, 1);
9372b35e1 Rusty Russell 2014-12-08 65 mutex_unlock(&reading_mutex);
d3cc79964 Amit Shah 2014-07-10 66 if (bytes_read > 0)
6d4952d9d Andrew Lutomirski 2016-10-17 67 add_device_randomness(rng_buffer, bytes_read);
d3cc79964 Amit Shah 2014-07-10 68 }
d3cc79964 Amit Shah 2014-07-10 69
3a2c0ba5a Rusty Russell 2014-12-08 70 static inline void cleanup_rng(struct kref *kref)
3a2c0ba5a Rusty Russell 2014-12-08 71 {
3a2c0ba5a Rusty Russell 2014-12-08 72 struct hwrng *rng = container_of(kref, struct hwrng, ref);
3a2c0ba5a Rusty Russell 2014-12-08 73
3a2c0ba5a Rusty Russell 2014-12-08 74 if (rng->cleanup)
3a2c0ba5a Rusty Russell 2014-12-08 75 rng->cleanup(rng);
a027f30d7 Rusty Russell 2014-12-08 76
77584ee57 Herbert Xu 2014-12-23 77 complete(&rng->cleanup_done);
3a2c0ba5a Rusty Russell 2014-12-08 78 }
3a2c0ba5a Rusty Russell 2014-12-08 79
90ac41bd4 Herbert Xu 2014-12-23 80 static int set_current_rng(struct hwrng *rng)
3a2c0ba5a Rusty Russell 2014-12-08 81 {
90ac41bd4 Herbert Xu 2014-12-23 82 int err;
90ac41bd4 Herbert Xu 2014-12-23 83
3a2c0ba5a Rusty Russell 2014-12-08 84 BUG_ON(!mutex_is_locked(&rng_mutex));
90ac41bd4 Herbert Xu 2014-12-23 85
90ac41bd4 Herbert Xu 2014-12-23 86 err = hwrng_init(rng);
90ac41bd4 Herbert Xu 2014-12-23 87 if (err)
90ac41bd4 Herbert Xu 2014-12-23 88 return err;
90ac41bd4 Herbert Xu 2014-12-23 89
ff77c150f Herbert Xu 2014-12-23 90 drop_current_rng();
3a2c0ba5a Rusty Russell 2014-12-08 91 current_rng = rng;
90ac41bd4 Herbert Xu 2014-12-23 92
90ac41bd4 Herbert Xu 2014-12-23 93 return 0;
3a2c0ba5a Rusty Russell 2014-12-08 94 }
3a2c0ba5a Rusty Russell 2014-12-08 95
3a2c0ba5a Rusty Russell 2014-12-08 96 static void drop_current_rng(void)
3a2c0ba5a Rusty Russell 2014-12-08 97 {
3a2c0ba5a Rusty Russell 2014-12-08 98 BUG_ON(!mutex_is_locked(&rng_mutex));
3a2c0ba5a Rusty Russell 2014-12-08 99 if (!current_rng)
3a2c0ba5a Rusty Russell 2014-12-08 100 return;
3a2c0ba5a Rusty Russell 2014-12-08 101
3a2c0ba5a Rusty Russell 2014-12-08 102 /* decrease last reference for triggering the cleanup */
3a2c0ba5a Rusty Russell 2014-12-08 103 kref_put(¤t_rng->ref, cleanup_rng);
3a2c0ba5a Rusty Russell 2014-12-08 104 current_rng = NULL;
3a2c0ba5a Rusty Russell 2014-12-08 105 }
3a2c0ba5a Rusty Russell 2014-12-08 106
3a2c0ba5a Rusty Russell 2014-12-08 107 /* Returns ERR_PTR(), NULL or refcounted hwrng */
3a2c0ba5a Rusty Russell 2014-12-08 108 static struct hwrng *get_current_rng(void)
3a2c0ba5a Rusty Russell 2014-12-08 109 {
3a2c0ba5a Rusty Russell 2014-12-08 110 struct hwrng *rng;
3a2c0ba5a Rusty Russell 2014-12-08 111
3a2c0ba5a Rusty Russell 2014-12-08 112 if (mutex_lock_interruptible(&rng_mutex))
3a2c0ba5a Rusty Russell 2014-12-08 113 return ERR_PTR(-ERESTARTSYS);
3a2c0ba5a Rusty Russell 2014-12-08 114
3a2c0ba5a Rusty Russell 2014-12-08 115 rng = current_rng;
3a2c0ba5a Rusty Russell 2014-12-08 116 if (rng)
3a2c0ba5a Rusty Russell 2014-12-08 117 kref_get(&rng->ref);
3a2c0ba5a Rusty Russell 2014-12-08 118
3a2c0ba5a Rusty Russell 2014-12-08 119 mutex_unlock(&rng_mutex);
3a2c0ba5a Rusty Russell 2014-12-08 120 return rng;
3a2c0ba5a Rusty Russell 2014-12-08 121 }
3a2c0ba5a Rusty Russell 2014-12-08 122
3a2c0ba5a Rusty Russell 2014-12-08 123 static void put_rng(struct hwrng *rng)
3a2c0ba5a Rusty Russell 2014-12-08 124 {
3a2c0ba5a Rusty Russell 2014-12-08 125 /*
3a2c0ba5a Rusty Russell 2014-12-08 126 * Hold rng_mutex here so we serialize in case they set_current_rng
3a2c0ba5a Rusty Russell 2014-12-08 127 * on rng again immediately.
3a2c0ba5a Rusty Russell 2014-12-08 128 */
3a2c0ba5a Rusty Russell 2014-12-08 129 mutex_lock(&rng_mutex);
3a2c0ba5a Rusty Russell 2014-12-08 130 if (rng)
3a2c0ba5a Rusty Russell 2014-12-08 131 kref_put(&rng->ref, cleanup_rng);
3a2c0ba5a Rusty Russell 2014-12-08 132 mutex_unlock(&rng_mutex);
3a2c0ba5a Rusty Russell 2014-12-08 133 }
3a2c0ba5a Rusty Russell 2014-12-08 134
90ac41bd4 Herbert Xu 2014-12-23 135 static int hwrng_init(struct hwrng *rng)
844dd05fe Michael Buesch 2006-06-26 136 {
15b66cd54 Herbert Xu 2014-12-23 137 if (kref_get_unless_zero(&rng->ref))
15b66cd54 Herbert Xu 2014-12-23 138 goto skip_init;
15b66cd54 Herbert Xu 2014-12-23 139
d3cc79964 Amit Shah 2014-07-10 140 if (rng->init) {
d3cc79964 Amit Shah 2014-07-10 141 int ret;
d3cc79964 Amit Shah 2014-07-10 142
d3cc79964 Amit Shah 2014-07-10 143 ret = rng->init(rng);
d3cc79964 Amit Shah 2014-07-10 144 if (ret)
d3cc79964 Amit Shah 2014-07-10 145 return ret;
d3cc79964 Amit Shah 2014-07-10 146 }
15b66cd54 Herbert Xu 2014-12-23 147
15b66cd54 Herbert Xu 2014-12-23 148 kref_init(&rng->ref);
15b66cd54 Herbert Xu 2014-12-23 149 reinit_completion(&rng->cleanup_done);
15b66cd54 Herbert Xu 2014-12-23 150
15b66cd54 Herbert Xu 2014-12-23 151 skip_init:
d3cc79964 Amit Shah 2014-07-10 152 add_early_randomness(rng);
be4000bc4 Torsten Duwe 2014-06-14 153
0f734e6e7 Torsten Duwe 2014-06-14 154 current_quality = rng->quality ? : default_quality;
506bf0c04 Keith Packard 2015-03-18 155 if (current_quality > 1024)
506bf0c04 Keith Packard 2015-03-18 156 current_quality = 1024;
0f734e6e7 Torsten Duwe 2014-06-14 157
0f734e6e7 Torsten Duwe 2014-06-14 158 if (current_quality == 0 && hwrng_fill)
0f734e6e7 Torsten Duwe 2014-06-14 159 kthread_stop(hwrng_fill);
be4000bc4 Torsten Duwe 2014-06-14 160 if (current_quality > 0 && !hwrng_fill)
be4000bc4 Torsten Duwe 2014-06-14 161 start_khwrngd();
be4000bc4 Torsten Duwe 2014-06-14 162
844dd05fe Michael Buesch 2006-06-26 163 return 0;
844dd05fe Michael Buesch 2006-06-26 164 }
844dd05fe Michael Buesch 2006-06-26 165
844dd05fe Michael Buesch 2006-06-26 166 static int rng_dev_open(struct inode *inode, struct file *filp)
844dd05fe Michael Buesch 2006-06-26 167 {
844dd05fe Michael Buesch 2006-06-26 168 /* enforce read-only access to this chrdev */
844dd05fe Michael Buesch 2006-06-26 @169 if ((filp->f_mode & FMODE_READ) == 0)
844dd05fe Michael Buesch 2006-06-26 170 return -EINVAL;
844dd05fe Michael Buesch 2006-06-26 @171 if (filp->f_mode & FMODE_WRITE)
844dd05fe Michael Buesch 2006-06-26 172 return -EINVAL;
844dd05fe Michael Buesch 2006-06-26 173 return 0;
844dd05fe Michael Buesch 2006-06-26 174 }
844dd05fe Michael Buesch 2006-06-26 175
9996508b3 Ian Molton 2009-12-01 176 static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
9996508b3 Ian Molton 2009-12-01 177 int wait) {
9996508b3 Ian Molton 2009-12-01 178 int present;
9996508b3 Ian Molton 2009-12-01 179
9372b35e1 Rusty Russell 2014-12-08 180 BUG_ON(!mutex_is_locked(&reading_mutex));
9996508b3 Ian Molton 2009-12-01 181 if (rng->read)
9996508b3 Ian Molton 2009-12-01 182 return rng->read(rng, (void *)buffer, size, wait);
9996508b3 Ian Molton 2009-12-01 183
9996508b3 Ian Molton 2009-12-01 184 if (rng->data_present)
9996508b3 Ian Molton 2009-12-01 185 present = rng->data_present(rng, wait);
9996508b3 Ian Molton 2009-12-01 186 else
9996508b3 Ian Molton 2009-12-01 187 present = 1;
9996508b3 Ian Molton 2009-12-01 188
9996508b3 Ian Molton 2009-12-01 189 if (present)
9996508b3 Ian Molton 2009-12-01 190 return rng->data_read(rng, (u32 *)buffer);
9996508b3 Ian Molton 2009-12-01 191
9996508b3 Ian Molton 2009-12-01 192 return 0;
9996508b3 Ian Molton 2009-12-01 193 }
9996508b3 Ian Molton 2009-12-01 194
844dd05fe Michael Buesch 2006-06-26 195 static ssize_t rng_dev_read(struct file *filp, char __user *buf,
844dd05fe Michael Buesch 2006-06-26 196 size_t size, loff_t *offp)
844dd05fe Michael Buesch 2006-06-26 197 {
844dd05fe Michael Buesch 2006-06-26 198 ssize_t ret = 0;
984e976f5 Patrick McHardy 2007-11-21 199 int err = 0;
9996508b3 Ian Molton 2009-12-01 200 int bytes_read, len;
3a2c0ba5a Rusty Russell 2014-12-08 201 struct hwrng *rng;
844dd05fe Michael Buesch 2006-06-26 202
844dd05fe Michael Buesch 2006-06-26 203 while (size) {
3a2c0ba5a Rusty Russell 2014-12-08 204 rng = get_current_rng();
3a2c0ba5a Rusty Russell 2014-12-08 205 if (IS_ERR(rng)) {
3a2c0ba5a Rusty Russell 2014-12-08 206 err = PTR_ERR(rng);
844dd05fe Michael Buesch 2006-06-26 207 goto out;
9996508b3 Ian Molton 2009-12-01 208 }
3a2c0ba5a Rusty Russell 2014-12-08 209 if (!rng) {
844dd05fe Michael Buesch 2006-06-26 210 err = -ENODEV;
3a2c0ba5a Rusty Russell 2014-12-08 211 goto out;
844dd05fe Michael Buesch 2006-06-26 212 }
984e976f5 Patrick McHardy 2007-11-21 213
1ab87298c Jiri Slaby 2015-11-27 214 if (mutex_lock_interruptible(&reading_mutex)) {
1ab87298c Jiri Slaby 2015-11-27 215 err = -ERESTARTSYS;
1ab87298c Jiri Slaby 2015-11-27 216 goto out_put;
1ab87298c Jiri Slaby 2015-11-27 217 }
9996508b3 Ian Molton 2009-12-01 218 if (!data_avail) {
3a2c0ba5a Rusty Russell 2014-12-08 219 bytes_read = rng_get_data(rng, rng_buffer,
f7f154f12 Rusty Russell 2013-03-05 220 rng_buffer_size(),
9996508b3 Ian Molton 2009-12-01 @221 !(filp->f_flags & O_NONBLOCK));
893f11286 Ralph Wuerthner 2008-04-17 222 if (bytes_read < 0) {
893f11286 Ralph Wuerthner 2008-04-17 223 err = bytes_read;
9372b35e1 Rusty Russell 2014-12-08 224 goto out_unlock_reading;
9996508b3 Ian Molton 2009-12-01 225 }
9996508b3 Ian Molton 2009-12-01 226 data_avail = bytes_read;
893f11286 Ralph Wuerthner 2008-04-17 227 }
844dd05fe Michael Buesch 2006-06-26 228
9996508b3 Ian Molton 2009-12-01 229 if (!data_avail) {
9996508b3 Ian Molton 2009-12-01 230 if (filp->f_flags & O_NONBLOCK) {
9996508b3 Ian Molton 2009-12-01 231 err = -EAGAIN;
9372b35e1 Rusty Russell 2014-12-08 232 goto out_unlock_reading;
9996508b3 Ian Molton 2009-12-01 233 }
9996508b3 Ian Molton 2009-12-01 234 } else {
9996508b3 Ian Molton 2009-12-01 235 len = data_avail;
9996508b3 Ian Molton 2009-12-01 236 if (len > size)
9996508b3 Ian Molton 2009-12-01 237 len = size;
9996508b3 Ian Molton 2009-12-01 238
9996508b3 Ian Molton 2009-12-01 239 data_avail -= len;
9996508b3 Ian Molton 2009-12-01 240
9996508b3 Ian Molton 2009-12-01 241 if (copy_to_user(buf + ret, rng_buffer + data_avail,
9996508b3 Ian Molton 2009-12-01 242 len)) {
844dd05fe Michael Buesch 2006-06-26 243 err = -EFAULT;
9372b35e1 Rusty Russell 2014-12-08 244 goto out_unlock_reading;
9996508b3 Ian Molton 2009-12-01 245 }
9996508b3 Ian Molton 2009-12-01 246
9996508b3 Ian Molton 2009-12-01 247 size -= len;
9996508b3 Ian Molton 2009-12-01 248 ret += len;
844dd05fe Michael Buesch 2006-06-26 249 }
844dd05fe Michael Buesch 2006-06-26 250
9372b35e1 Rusty Russell 2014-12-08 251 mutex_unlock(&reading_mutex);
3a2c0ba5a Rusty Russell 2014-12-08 252 put_rng(rng);
9996508b3 Ian Molton 2009-12-01 253
844dd05fe Michael Buesch 2006-06-26 254 if (need_resched())
844dd05fe Michael Buesch 2006-06-26 255 schedule_timeout_interruptible(1);
9996508b3 Ian Molton 2009-12-01 256
9996508b3 Ian Molton 2009-12-01 257 if (signal_pending(current)) {
844dd05fe Michael Buesch 2006-06-26 258 err = -ERESTARTSYS;
844dd05fe Michael Buesch 2006-06-26 259 goto out;
844dd05fe Michael Buesch 2006-06-26 260 }
9996508b3 Ian Molton 2009-12-01 261 }
844dd05fe Michael Buesch 2006-06-26 262 out:
844dd05fe Michael Buesch 2006-06-26 263 return ret ? : err;
3a2c0ba5a Rusty Russell 2014-12-08 264
9372b35e1 Rusty Russell 2014-12-08 265 out_unlock_reading:
9372b35e1 Rusty Russell 2014-12-08 266 mutex_unlock(&reading_mutex);
1ab87298c Jiri Slaby 2015-11-27 267 out_put:
3a2c0ba5a Rusty Russell 2014-12-08 268 put_rng(rng);
3a2c0ba5a Rusty Russell 2014-12-08 269 goto out;
844dd05fe Michael Buesch 2006-06-26 270 }
844dd05fe Michael Buesch 2006-06-26 271
62322d255 Arjan van de Ven 2006-07-03 @272 static const struct file_operations rng_chrdev_ops = {
844dd05fe Michael Buesch 2006-06-26 @273 .owner = THIS_MODULE,
844dd05fe Michael Buesch 2006-06-26 @274 .open = rng_dev_open,
844dd05fe Michael Buesch 2006-06-26 @275 .read = rng_dev_read,
6038f373a Arnd Bergmann 2010-08-15 @276 .llseek = noop_llseek,
844dd05fe Michael Buesch 2006-06-26 277 };
844dd05fe Michael Buesch 2006-06-26 278
0daa7a0af Takashi Iwai 2015-02-02 279 static const struct attribute_group *rng_dev_groups[];
:::::: The code at line 169 was first introduced by commit
:::::: 844dd05fec172d98b0dacecd9b9e9f6595204c13 [PATCH] Add new generic HW RNG core
:::::: TO: Michael Buesch <mb@bu3sch.de>
:::::: CC: Linus Torvalds <torvalds@g5.osdl.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 17496 bytes --]
^ permalink raw reply
* Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Andy Lutomirski @ 2016-12-10 5:25 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-crypto, linux-kernel@vger.kernel.org, linux-mm@kvack.org,
kernel-hardening@lists.openwall.com, Herbert Xu,
Andrew Lutomirski, Stephan Mueller
In-Reply-To: <20161209230851.GB64048@google.com>
On Fri, Dec 9, 2016 at 3:08 PM, Eric Biggers <ebiggers3@gmail.com> wrote:
> In the 4.9 kernel, virtually-mapped stacks will be supported and enabled by
> default on x86_64. This has been exposing a number of problems in which
> on-stack buffers are being passed into the crypto API, which to support crypto
> accelerators operates on 'struct page' rather than on virtual memory.
>
> Some of these problems have already been fixed, but I was wondering how many
> problems remain, so I briefly looked through all the callers of sg_set_buf() and
> sg_init_one(). Overall I found quite a few remaining problems, detailed below.
>
> The following crypto drivers initialize a scatterlist to point into an
> ahash_request, which may have been allocated on the stack with
> AHASH_REQUEST_ON_STACK():
>
> drivers/crypto/bfin_crc.c:351
> drivers/crypto/qce/sha.c:299
> drivers/crypto/sahara.c:973,988
> drivers/crypto/talitos.c:1910
This are impossible or highly unlikely on x86.
> drivers/crypto/ccp/ccp-crypto-aes-cmac.c:105,119,142
> drivers/crypto/ccp/ccp-crypto-sha.c:95,109,124
These
> drivers/crypto/qce/sha.c:325
This is impossible on x86.
>
> The following crypto drivers initialize a scatterlist to point into an
> ablkcipher_request, which may have been allocated on the stack with
> SKCIPHER_REQUEST_ON_STACK():
>
> drivers/crypto/ccp/ccp-crypto-aes-xts.c:162
> drivers/crypto/ccp/ccp-crypto-aes.c:94
These are real, and I wish I'd known about them sooner.
>
> And these other places do crypto operations on buffers clearly on the stack:
>
> drivers/net/wireless/intersil/orinoco/mic.c:72
Ick.
> drivers/usb/wusbcore/crypto.c:264
Well, crud. I thought I had fixed this driver but I missed one case.
Will send a fix tomorrow. But I'm still unconvinced that this
hardware ever shipped.
> net/ceph/crypto.c:182
Ick.
> net/rxrpc/rxkad.c:737,1000
Well, crud. This was supposed to have been fixed in:
commit a263629da519b2064588377416e067727e2cbdf9
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sun Jun 26 14:55:24 2016 -0700
rxrpc: Avoid using stack memory in SG lists in rxkad
> security/keys/encrypted-keys/encrypted.c:500
That's a trivial one-liner. Patch coming tomorrow.
> fs/cifs/smbencrypt.c:96
Ick.
>
> Note: I almost certainly missed some, since I excluded places where the use of a
> stack buffer was not obvious to me. I also excluded AEAD algorithms since there
> isn't an AEAD_REQUEST_ON_STACK() macro (yet).
>
> The "good" news with these bugs is that on x86_64 without CONFIG_DEBUG_SG=y or
> CONFIG_DEBUG_VIRTUAL=y, you can still do virt_to_page() and then page_address()
> on a vmalloc address and get back the same address, even though you aren't
> *supposed* to be able to do this. This will make things still work for most
> people. The bad news is that if you happen to have consumed just about 1 page
> (or N pages) of your stack at the time you call the crypto API, your stack
> buffer may actually span physically non-contiguous pages, so the crypto
> algorithm will scribble over some unrelated page.
Are you sure? If it round-trips to the same virtual address, it
doesn't matter if the buffer is contiguous.
> Also, hardware crypto drivers
> which actually do operate on physical memory will break too.
Those were already broken. DMA has been illegal on the stack for
years and DMA debugging would have caught it.
>
> So I am wondering: is the best solution really to make all these crypto API
> algorithms and users use heap buffers, as opposed to something like maintaining
> a lowmem alias for the stack, or introducing a more general function to convert
> buffers (possibly in the vmalloc space) into scatterlists? And if the current
> solution is desired, who is going to fix all of these bugs and when?
The *right* solution IMO is to fix crypto to stop using scatterlists.
Scatterlists are for DMA using physical addresses, and they're
inappropriate almost every user of them that's using them for crypto.
kiov would be much better -- it would make sense and it would be
faster.
I have a hack to make scatterlists pointing to the stack work (as long
as they're only one element), but that's seriously gross.
Herbert, how hard would it be to teach the crypto code to use a more
sensible data structure than scatterlist and to use coccinelle fix
this stuff for real?
In the mean time, we should patch the handful of drivers that matter.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 7/7] hwrng: core: Remove two unused include
From: kbuild test robot @ 2016-12-10 5:31 UTC (permalink / raw)
To: Corentin Labbe
Cc: kbuild-all, mpm, herbert, arnd, gregkh, linux-crypto,
linux-kernel, Corentin Labbe
In-Reply-To: <1481293299-21697-7-git-send-email-clabbe.montjoie@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5431 bytes --]
Hi Corentin,
[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v4.9-rc8 next-20161209]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Corentin-Labbe/hwrng-core-do-not-use-multiple-blank-lines/20161210-072632
config: i386-randconfig-i0-201649 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/char/hw_random/core.c: In function 'rng_dev_open':
>> drivers/char/hw_random/core.c:169:11: error: dereferencing pointer to incomplete type
if ((filp->f_mode & FMODE_READ) == 0)
^
drivers/char/hw_random/core.c:169:22: error: 'FMODE_READ' undeclared (first use in this function)
if ((filp->f_mode & FMODE_READ) == 0)
^
drivers/char/hw_random/core.c:169:22: note: each undeclared identifier is reported only once for each function it appears in
drivers/char/hw_random/core.c:171:10: error: dereferencing pointer to incomplete type
if (filp->f_mode & FMODE_WRITE)
^
drivers/char/hw_random/core.c:171:21: error: 'FMODE_WRITE' undeclared (first use in this function)
if (filp->f_mode & FMODE_WRITE)
^
drivers/char/hw_random/core.c: In function 'rng_dev_read':
drivers/char/hw_random/core.c:221:11: error: dereferencing pointer to incomplete type
!(filp->f_flags & O_NONBLOCK));
^
drivers/char/hw_random/core.c:221:23: error: 'O_NONBLOCK' undeclared (first use in this function)
!(filp->f_flags & O_NONBLOCK));
^
drivers/char/hw_random/core.c:230:12: error: dereferencing pointer to incomplete type
if (filp->f_flags & O_NONBLOCK) {
^
drivers/char/hw_random/core.c: At top level:
drivers/char/hw_random/core.c:272:21: error: variable 'rng_chrdev_ops' has initializer but incomplete type
static const struct file_operations rng_chrdev_ops = {
^
drivers/char/hw_random/core.c:273:2: error: unknown field 'owner' specified in initializer
.owner = THIS_MODULE,
^
In file included from include/linux/linkage.h:6:0,
from include/linux/kernel.h:6,
from include/linux/delay.h:10,
from drivers/char/hw_random/core.c:13:
include/linux/export.h:37:30: warning: excess elements in struct initializer [enabled by default]
#define THIS_MODULE ((struct module *)0)
^
drivers/char/hw_random/core.c:273:12: note: in expansion of macro 'THIS_MODULE'
.owner = THIS_MODULE,
^
include/linux/export.h:37:30: warning: (near initialization for 'rng_chrdev_ops') [enabled by default]
#define THIS_MODULE ((struct module *)0)
^
drivers/char/hw_random/core.c:273:12: note: in expansion of macro 'THIS_MODULE'
.owner = THIS_MODULE,
^
drivers/char/hw_random/core.c:274:2: error: unknown field 'open' specified in initializer
.open = rng_dev_open,
^
drivers/char/hw_random/core.c:274:2: warning: excess elements in struct initializer [enabled by default]
drivers/char/hw_random/core.c:274:2: warning: (near initialization for 'rng_chrdev_ops') [enabled by default]
drivers/char/hw_random/core.c:275:2: error: unknown field 'read' specified in initializer
.read = rng_dev_read,
^
drivers/char/hw_random/core.c:275:2: warning: excess elements in struct initializer [enabled by default]
drivers/char/hw_random/core.c:275:2: warning: (near initialization for 'rng_chrdev_ops') [enabled by default]
drivers/char/hw_random/core.c:276:2: error: unknown field 'llseek' specified in initializer
.llseek = noop_llseek,
^
drivers/char/hw_random/core.c:276:13: error: 'noop_llseek' undeclared here (not in a function)
.llseek = noop_llseek,
^
drivers/char/hw_random/core.c:276:2: warning: excess elements in struct initializer [enabled by default]
.llseek = noop_llseek,
^
drivers/char/hw_random/core.c:276:2: warning: (near initialization for 'rng_chrdev_ops') [enabled by default]
vim +169 drivers/char/hw_random/core.c
844dd05f Michael Buesch 2006-06-26 163 return 0;
844dd05f Michael Buesch 2006-06-26 164 }
844dd05f Michael Buesch 2006-06-26 165
844dd05f Michael Buesch 2006-06-26 166 static int rng_dev_open(struct inode *inode, struct file *filp)
844dd05f Michael Buesch 2006-06-26 167 {
844dd05f Michael Buesch 2006-06-26 168 /* enforce read-only access to this chrdev */
844dd05f Michael Buesch 2006-06-26 @169 if ((filp->f_mode & FMODE_READ) == 0)
844dd05f Michael Buesch 2006-06-26 170 return -EINVAL;
844dd05f Michael Buesch 2006-06-26 171 if (filp->f_mode & FMODE_WRITE)
844dd05f Michael Buesch 2006-06-26 172 return -EINVAL;
:::::: The code at line 169 was first introduced by commit
:::::: 844dd05fec172d98b0dacecd9b9e9f6595204c13 [PATCH] Add new generic HW RNG core
:::::: TO: Michael Buesch <mb@bu3sch.de>
:::::: CC: Linus Torvalds <torvalds@g5.osdl.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26603 bytes --]
^ permalink raw reply
* Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Herbert Xu @ 2016-12-10 5:32 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Eric Biggers, linux-crypto, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, kernel-hardening@lists.openwall.com,
Andrew Lutomirski, Stephan Mueller
In-Reply-To: <CALCETrW=+3u3P8Xva+0ck9=fr-mD6azPtTkOQ3uQO+GoOA6FcQ@mail.gmail.com>
On Fri, Dec 09, 2016 at 09:25:38PM -0800, Andy Lutomirski wrote:
>
> > The following crypto drivers initialize a scatterlist to point into an
> > ablkcipher_request, which may have been allocated on the stack with
> > SKCIPHER_REQUEST_ON_STACK():
> >
> > drivers/crypto/ccp/ccp-crypto-aes-xts.c:162
> > drivers/crypto/ccp/ccp-crypto-aes.c:94
>
> These are real, and I wish I'd known about them sooner.
Are you sure? Any instance of *_ON_STACK must only be used with
sync algorithms and most drivers under drivers/crypto declare
themselves as async.
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
* Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Herbert Xu @ 2016-12-10 5:37 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Eric Biggers, linux-crypto, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, kernel-hardening@lists.openwall.com,
Andrew Lutomirski, Stephan Mueller
In-Reply-To: <CALCETrW=+3u3P8Xva+0ck9=fr-mD6azPtTkOQ3uQO+GoOA6FcQ@mail.gmail.com>
On Fri, Dec 09, 2016 at 09:25:38PM -0800, Andy Lutomirski wrote:
>
> Herbert, how hard would it be to teach the crypto code to use a more
> sensible data structure than scatterlist and to use coccinelle fix
> this stuff for real?
First of all we already have a sync non-SG hash interface, it's
called shash.
If we had enough sync-only users of skcipher then I'll consider
adding an interface for it. However, at this point in time it
appears to more sense to convert such users over to the async
interface rather than the other way around.
As for AEAD we never had a sync interface to begin with and I
don't think I'm going to add one.
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
* Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Eric Biggers @ 2016-12-10 5:55 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-crypto, linux-kernel@vger.kernel.org, linux-mm@kvack.org,
kernel-hardening@lists.openwall.com, Herbert Xu,
Andrew Lutomirski, Stephan Mueller
In-Reply-To: <CALCETrW=+3u3P8Xva+0ck9=fr-mD6azPtTkOQ3uQO+GoOA6FcQ@mail.gmail.com>
On Fri, Dec 09, 2016 at 09:25:38PM -0800, Andy Lutomirski wrote:
> > The following crypto drivers initialize a scatterlist to point into an
> > ahash_request, which may have been allocated on the stack with
> > AHASH_REQUEST_ON_STACK():
> >
> > drivers/crypto/bfin_crc.c:351
> > drivers/crypto/qce/sha.c:299
> > drivers/crypto/sahara.c:973,988
> > drivers/crypto/talitos.c:1910
>
> This are impossible or highly unlikely on x86.
>
> > drivers/crypto/ccp/ccp-crypto-aes-cmac.c:105,119,142
> > drivers/crypto/ccp/ccp-crypto-sha.c:95,109,124
>
> These
>
> > drivers/crypto/qce/sha.c:325
>
> This is impossible on x86.
>
Thanks for looking into these. I didn't investigate who/what is likely to be
using each driver.
Of course I would not be surprised to see people want to start supporting
virtually mapped stacks on other architectures too.
> >
> > The "good" news with these bugs is that on x86_64 without CONFIG_DEBUG_SG=y or
> > CONFIG_DEBUG_VIRTUAL=y, you can still do virt_to_page() and then page_address()
> > on a vmalloc address and get back the same address, even though you aren't
> > *supposed* to be able to do this. This will make things still work for most
> > people. The bad news is that if you happen to have consumed just about 1 page
> > (or N pages) of your stack at the time you call the crypto API, your stack
> > buffer may actually span physically non-contiguous pages, so the crypto
> > algorithm will scribble over some unrelated page.
>
> Are you sure? If it round-trips to the same virtual address, it
> doesn't matter if the buffer is contiguous.
You may be right, I didn't test this. The hash_walk and blkcipher_walk code do
go page by page, but I suppose on x86_64 it would just step from one bogus
"struct page" to the adjacent one and still map it to the original virtual
address.
Eric
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Crypto Fixes for 4.9
From: Herbert Xu @ 2016-12-10 6:01 UTC (permalink / raw)
To: Linus Torvalds, David S. Miller, Linux Kernel Mailing List,
Linux Crypto Mailing List
In-Reply-To: <20161205063754.GA9408@gondor.apana.org.au>
Hi Linus:
This push fixes the following issues:
- Fix pointer size when caam is used with AArch64 boot loader on
AArch32 kernel.
- Fix ahash state corruption in marvell driver.
- Fix buggy algif_aed tag handling.
- Prevent mcryptd from being used with incompatible algorithms
which can cause crashes.
Please pull from
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git linus
Horia Geantă (1):
crypto: caam - fix pointer size for AArch64 boot loader, AArch32 kernel
Romain Perier (2):
crypto: marvell - Don't copy hash operation twice into the SRAM
crypto: marvell - Don't corrupt state of an STD req for re-stepped ahash
Stephan Mueller (2):
crypto: algif_aead - fix AEAD tag memory handling
crypto: algif_aead - fix uninitialized variable warning
tim (1):
crypto: mcryptd - Check mcryptd algorithm compatibility
crypto/algif_aead.c | 59 ++++++++++++++++++++++++++---------------
crypto/mcryptd.c | 19 ++++++++-----
drivers/crypto/caam/ctrl.c | 5 ++--
drivers/crypto/marvell/hash.c | 11 ++++----
4 files changed, 57 insertions(+), 37 deletions(-)
Thanks,
--
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
* Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Eric Biggers @ 2016-12-10 6:03 UTC (permalink / raw)
To: kernel-hardening
Cc: Andy Lutomirski, linux-crypto, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, Andrew Lutomirski, Stephan Mueller
In-Reply-To: <20161210053208.GA27951@gondor.apana.org.au>
On Sat, Dec 10, 2016 at 01:32:08PM +0800, Herbert Xu wrote:
> On Fri, Dec 09, 2016 at 09:25:38PM -0800, Andy Lutomirski wrote:
> >
> > > The following crypto drivers initialize a scatterlist to point into an
> > > ablkcipher_request, which may have been allocated on the stack with
> > > SKCIPHER_REQUEST_ON_STACK():
> > >
> > > drivers/crypto/ccp/ccp-crypto-aes-xts.c:162
> > > drivers/crypto/ccp/ccp-crypto-aes.c:94
> >
> > These are real, and I wish I'd known about them sooner.
>
> Are you sure? Any instance of *_ON_STACK must only be used with
> sync algorithms and most drivers under drivers/crypto declare
> themselves as async.
>
Why exactly is that? Obviously, it wouldn't work if you returned from the stack
frame before the request completed, but does anything stop someone from using an
*_ON_STACK() request and then waiting for the request to complete before
returning from the stack frame?
Eric
^ permalink raw reply
* Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Eric Biggers @ 2016-12-10 6:30 UTC (permalink / raw)
To: kernel-hardening
Cc: Andy Lutomirski, linux-crypto, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, Andrew Lutomirski, Stephan Mueller
In-Reply-To: <20161210053711.GB27951@gondor.apana.org.au>
On Sat, Dec 10, 2016 at 01:37:12PM +0800, Herbert Xu wrote:
> On Fri, Dec 09, 2016 at 09:25:38PM -0800, Andy Lutomirski wrote:
> >
> > Herbert, how hard would it be to teach the crypto code to use a more
> > sensible data structure than scatterlist and to use coccinelle fix
> > this stuff for real?
>
> First of all we already have a sync non-SG hash interface, it's
> called shash.
>
> If we had enough sync-only users of skcipher then I'll consider
> adding an interface for it. However, at this point in time it
> appears to more sense to convert such users over to the async
> interface rather than the other way around.
>
> As for AEAD we never had a sync interface to begin with and I
> don't think I'm going to add one.
>
Isn't the question of "should the API use physical or virtual addresses"
independent of the question of "should the API support asynchronous requests"?
You can already choose, via the flags and mask arguments when allocating a
crypto transform, whether you want it to be synchronous or asynchronous or
whether you don't care. I don't see what that says about whether the API should
take in physical memory (e.g. scatterlists or struct pages) or virtual memory
(e.g. iov_iters or just regular pointers).
And while it's true that asynchronous algorithms are often provided by hardware
drivers that operate on physical memory, it's not always the case. For example
some of the AES-NI algorithms are asynchronous only because they use the SSE
registers which can't always available to kernel code, so the request may need
to be processed by another thread.
Eric
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: scatterwalk_map_and_copy incorrect optimization
From: Herbert Xu @ 2016-12-10 8:08 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: linux-crypto, LKML
In-Reply-To: <CAHmME9p24bkNPD2CfGMUOHxgOX+OuFO_rVqfhOQdDexwW53ExA@mail.gmail.com>
On Fri, Dec 09, 2016 at 02:18:01PM +0100, Jason A. Donenfeld wrote:
> Hi Herbert,
>
> The scatterwalk_map_and_copy function copies ordinary buffers to and
> from scatterlists. These buffers can, of course, be on the stack, and
> this remains the most popular use of this function -- getting info
> between stack buffers and DMA regions. It's mostly used for adding or
> checking MACs, in the majority of call sites. Its implementation is
> relatively straightforward. It maps the DMA region(s) to a vaddr, and
> then just calls vanilla memcpy. Pretty uncontroversial.
>
> However, around ~4.1 an optimization was added to prevent copying when
> unnecessary (when the src and dst are the same). The optimization
> looks like this:
>
> if (sg_page(sg) == virt_to_page(buf) &&
> sg->offset == offset_in_page(buf))
> return;
>
> There are two problems with this:
This code no longer exists in the current tree.
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
* Re: algif for compression?
From: Herbert Xu @ 2016-12-10 8:10 UTC (permalink / raw)
To: abed mohammad kamaluddin; +Cc: linux-crypto
In-Reply-To: <CACVarETuTx40WFD5TRUCwiYv5r3SEvhuJHpGjqPsz90qVZsYKA@mail.gmail.com>
abed mohammad kamaluddin <abedamu@gmail.com> wrote:
>
> We are also looking for user-space access to the kernel crypto layer
> compression algorithms. I have gone through AF_ALG but couldn’t find
> support for compression ops through it. This is required for our
> hardware zip engines whose driver hooks up to the crypto layer.
>
> I was going through the lists and stumbled across this thread. Has
> there been any updates or efforts put up in this direction since this
> thread is a few years old. If not, are there any alternate
> implementations that allow user-space access to compression? I have
> gone through cryptodev and see the same limitation there.
>
> Would appreciate any pointers in this regard.
The compression interface is currently in a state of flux. We
should make it settle down first before exporting it to user-space.
For a start it would be good to actually switch IPsec/IPcomp over
to the new compression interface.
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
* Re: scatterwalk_map_and_copy incorrect optimization
From: Herbert Xu @ 2016-12-10 8:11 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: linux-crypto, ebiggers, linux-kernel
In-Reply-To: <CAHmME9rq9NjawkZw6wQpWsrY58ZgyRutBKT5UL7RA+8Pv2GOew@mail.gmail.com>
Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Hah, looks like I missed [1] by a couple weeks. Looks like it's been
> settled then.
>
> Is this a stable@ candidate?
Not really since it shouldn't cause any problems unless the stack
is vmalloced.
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
* Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Herbert Xu @ 2016-12-10 8:16 UTC (permalink / raw)
To: Eric Biggers
Cc: kernel-hardening, luto, linux-crypto, linux-kernel, linux-mm,
luto, smueller
In-Reply-To: <20161210060316.GC6846@zzz>
Why did you drop me from the CC list when you were replying to
my email?
Eric Biggers <ebiggers3@gmail.com> wrote:
> On Sat, Dec 10, 2016 at 01:32:08PM +0800, Herbert Xu wrote:
>
>> Are you sure? Any instance of *_ON_STACK must only be used with
>> sync algorithms and most drivers under drivers/crypto declare
>> themselves as async.
>
> Why exactly is that? Obviously, it wouldn't work if you returned from the stack
> frame before the request completed, but does anything stop someone from using an
> *_ON_STACK() request and then waiting for the request to complete before
> returning from the stack frame?
The *_ON_STACK variants (except SHASH of course) were simply hacks
to help legacy crypto API users to cope with the new async interface.
In general we should avoid using the sync interface when possible.
It's a bad idea for the obvious reason that most of our async
algorithms want to DMA and that doesn't work very well when you're
using memory from the stack.
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
* Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Eric Biggers @ 2016-12-10 8:39 UTC (permalink / raw)
To: Herbert Xu
Cc: kernel-hardening, luto, linux-crypto, linux-kernel, linux-mm,
luto, smueller
In-Reply-To: <20161210081643.GA384@gondor.apana.org.au>
On Sat, Dec 10, 2016 at 04:16:43PM +0800, Herbert Xu wrote:
> Why did you drop me from the CC list when you were replying to
> my email?
>
Sorry --- this thread is Cc'ed to the kernel-hardening mailing list (which was
somewhat recently revived), and I replied to the email that reached me from
there. It looks like it currently behaves a little differently from the vger
mailing lists, in that it replaces "Reply-To" with the address of the mailing
list itself rather than the sender. So that's how you got dropped. It also
seems to add a prefix to the subject...
I
> >> Are you sure? Any instance of *_ON_STACK must only be used with
> >> sync algorithms and most drivers under drivers/crypto declare
> >> themselves as async.
> >
> > Why exactly is that? Obviously, it wouldn't work if you returned from the stack
> > frame before the request completed, but does anything stop someone from using an
> > *_ON_STACK() request and then waiting for the request to complete before
> > returning from the stack frame?
>
> The *_ON_STACK variants (except SHASH of course) were simply hacks
> to help legacy crypto API users to cope with the new async interface.
> In general we should avoid using the sync interface when possible.
>
> It's a bad idea for the obvious reason that most of our async
> algorithms want to DMA and that doesn't work very well when you're
> using memory from the stack.
Sure, I just feel that the idea of "is this algorithm asynchronous?" is being
conflated with the idea of "does this algorithm operate on physical memory?".
Also, if *_ON_STACK are really not allowed with asynchronous algorithms can
there at least be a comment or a WARN_ON() to express this?
Thanks,
Eric
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* RE: [PATCH v6 1/2] sparc: fix a building error reported by kbuild
From: Gonglei (Arei) @ 2016-12-10 8:40 UTC (permalink / raw)
To: Sam Ravnborg
Cc: linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
virtio-dev@lists.oasis-open.org,
virtualization@lists.linux-foundation.org,
linux-crypto@vger.kernel.org, Luonengjun, mst@redhat.com,
stefanha@redhat.com, Huangweidong (C), Wubin (H),
xin.zeng@intel.com, Claudio Fontana, herbert@gondor.apana.org.au,
pasic@linux.vnet.ibm.com, davem@davemloft.net
In-Reply-To: <20161209215851.GA7717@ravnborg.org>
Regards,
-Gonglei
> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org
> [mailto:linux-crypto-owner@vger.kernel.org] On Behalf Of Sam Ravnborg
> Sent: Saturday, December 10, 2016 5:59 AM
> To: Gonglei (Arei)
> Cc: linux-kernel@vger.kernel.org; qemu-devel@nongnu.org;
> virtio-dev@lists.oasis-open.org; virtualization@lists.linux-foundation.org;
> linux-crypto@vger.kernel.org; Luonengjun; mst@redhat.com;
> stefanha@redhat.com; Huangweidong (C); Wubin (H); xin.zeng@intel.com;
> Claudio Fontana; herbert@gondor.apana.org.au; pasic@linux.vnet.ibm.com;
> davem@davemloft.net; Zhoujian (jay, Euler); Hanweidong (Randy);
> arei.gonglei@hotmail.com; cornelia.huck@de.ibm.com; Xuquan (Quan Xu);
> longpeng; Wanzongshun (Vincent); sparclinux@vger.kernel.org
> Subject: Re: [PATCH v6 1/2] sparc: fix a building error reported by kbuild
>
> Hi Gonglei.
>
> On Thu, Dec 08, 2016 at 12:37:08PM +0800, Gonglei wrote:
> > >> arch/sparc/include/asm/topology_64.h:44:44:
> > error: implicit declaration of function 'cpu_data'
> > [-Werror=implicit-function-declaration]
> >
> > #define topology_physical_package_id(cpu) (cpu_data(cpu).proc_id)
> > ^
> > Let's include cpudata.h in topology_64.h.
> >
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: sparclinux@vger.kernel.org
> > Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
Thanks.
> > ---
> > arch/sparc/include/asm/topology_64.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/sparc/include/asm/topology_64.h
> b/arch/sparc/include/asm/topology_64.h
> > index 7b4898a..2255430 100644
> > --- a/arch/sparc/include/asm/topology_64.h
> > +++ b/arch/sparc/include/asm/topology_64.h
> > @@ -4,6 +4,7 @@
> > #ifdef CONFIG_NUMA
> >
> > #include <asm/mmzone.h>
> > +#include <asm/cpudata.h>
>
> Nitpick - if you are going to resend this patch,
It depends on the maintainer's thought. :)
> then please order the two includes in alphabetic order.
>
> For two includes this looks like bikeshedding, but when we add
> more having them in a defined arder prevents merge conflicts.
> And makes it readable too.
>
> We also sometimes order the includes with the longest lines topmost,
> and lines with the ame length are ordered alphabetically.
> But this is not seen so often.
>
Regards,
-Gonglei
^ permalink raw reply
* Re: [kernel-hardening] [PATCH] siphash: add cryptographically secure hashtable function
From: Greg KH @ 2016-12-10 12:37 UTC (permalink / raw)
To: kernel-hardening
Cc: linux-kernel, linux-crypto, rusty, torvalds, Jason A. Donenfeld,
Jean-Philippe Aumasson, Daniel J . Bernstein
In-Reply-To: <20161209183659.25727-1-Jason@zx2c4.com>
On Fri, Dec 09, 2016 at 07:36:59PM +0100, Jason A. Donenfeld wrote:
> SipHash is a 64-bit keyed hash function that is actually a
> cryptographically secure PRF, like HMAC. Except SipHash is super fast,
> and is meant to be used as a hashtable keyed lookup function.
>
> SipHash isn't just some new trendy hash function. It's been around for a
> while, and there really isn't anything that comes remotely close to
> being useful in the way SipHash is. With that said, why do we need this?
>
> There are a variety of attacks known as "hashtable poisoning" in which an
> attacker forms some data such that the hash of that data will be the
> same, and then preceeds to fill up all entries of a hashbucket. This is
> a realistic and well-known denial-of-service vector.
>
> Linux developers already seem to be aware that this is an issue, and
> various places that use hash tables in, say, a network context, use a
> non-cryptographically secure function (usually jhash) and then try to
> twiddle with the key on a time basis (or in many cases just do nothing
> and hope that nobody notices). While this is an admirable attempt at
> solving the problem, it doesn't actually fix it. SipHash fixes it.
>
> (It fixes it in such a sound way that you could even build a stream
> cipher out of SipHash that would resist the modern cryptanalysis.)
>
> There are a modicum of places in the kernel that are vulnerable to
> hashtable poisoning attacks, either via userspace vectors or network
> vectors, and there's not a reliable mechanism inside the kernel at the
> moment to fix it. The first step toward fixing these issues is actually
> getting a secure primitive into the kernel for developers to use. Then
> we can, bit by bit, port things over to it as deemed appropriate.
>
> Dozens of languages are already using this internally for their hash
> tables. Some of the BSDs already use this in their kernels. SipHash is
> a widely known high-speed solution to a widely known problem, and it's
> time we catch-up.
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Cc: Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
> Cc: Daniel J. Bernstein <djb@cr.yp.to>
> ---
> include/linux/siphash.h | 18 ++++++
> lib/Makefile | 3 +-
> lib/siphash.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 183 insertions(+), 1 deletion(-)
> create mode 100644 include/linux/siphash.h
> create mode 100644 lib/siphash.c
This looks really nice, but we don't usually add stuff into lib/ unless
there is an actual user of the code :)
Have you tried converting any of the existing hash users to use this
instead? If you did that, and it shows a solution for the known
problems with our existing hashes (as you point out above), I doubt
there would be any objection for this patch at all.
Minor coding style nits below:
> @@ -0,0 +1,18 @@
> +/* Copyright (C) 2016 Jason A. Donenfeld <Jason@zx2c4.com>
> + *
> + * SipHash: a fast short-input PRF
> + * https://131002.net/siphash/
> + */
> +
> +#ifndef _LINUX_SIPHASH_H
> +#define _LINUX_SIPHASH_H
> +
> +#include <linux/types.h>
> +
> +enum siphash24_lengths {
> + SIPHASH24_KEY_LEN = 16
> +};
> +
> +uint64_t siphash24(const uint8_t *data, size_t len, const uint8_t key[SIPHASH24_KEY_LEN]);
Please use u64 and u8 instead of the userspace uint64_t and uint8_t
types for kernel code. Yes, the ship has probably sailed for trying to
strictly enforce it, but it's a good idea to do where ever possible.
> +
> +#endif /* _LINUX_SIPHASH_H */
> diff --git a/lib/Makefile b/lib/Makefile
> index 50144a3aeebd..d224337b0d01 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -22,7 +22,8 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
> sha1.o chacha20.o md5.o irq_regs.o argv_split.o \
> flex_proportions.o ratelimit.o show_mem.o \
> is_single_threaded.o plist.o decompress.o kobject_uevent.o \
> - earlycpio.o seq_buf.o nmi_backtrace.o nodemask.o win_minmax.o
> + earlycpio.o seq_buf.o siphash.o \
> + nmi_backtrace.o nodemask.o win_minmax.o
>
> lib-$(CONFIG_MMU) += ioremap.o
> lib-$(CONFIG_SMP) += cpumask.o
> diff --git a/lib/siphash.c b/lib/siphash.c
> new file mode 100644
> index 000000000000..022d86f04b9b
> --- /dev/null
> +++ b/lib/siphash.c
> @@ -0,0 +1,163 @@
> +/* Copyright (C) 2015-2016 Jason A. Donenfeld <Jason@zx2c4.com>
> + * Copyright (C) 2012-2014 Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
> + * Copyright (C) 2012-2014 Daniel J. Bernstein <djb@cr.yp.to>
> + *
> + * SipHash: a fast short-input PRF
> + * https://131002.net/siphash/
> + */
Any specific license for this code? It's good to at the least say what
it is. Yes, we know it will default to GPLv2 only as part of the whole
kernel tree, but it's good to be explicit for when someone wants to copy
this code for their own projects...
> +
> +#include <linux/siphash.h>
> +#include <linux/kernel.h>
> +#include <linux/string.h>
> +
> +#define ROTL(x,b) (uint64_t)(((x) << (b)) | ((x) >> (64 - (b))))
Don't we have this in kernel.h somewhere? Ah, yeah, it's rol64() in
bitops.h, no need to define it again please.
> +#define U8TO64(p) le64_to_cpu(*(__le64 *)(p))
Why the crazy casting behind a macro?
> +
> +#define SIPROUND \
> + do { \
> + v0 += v1; v1 = ROTL(v1, 13); v1 ^= v0; v0 = ROTL(v0, 32); \
> + v2 += v3; v3 = ROTL(v3, 16); v3 ^= v2; \
> + v0 += v3; v3 = ROTL(v3, 21); v3 ^= v0; \
> + v2 += v1; v1 = ROTL(v1, 17); v1 ^= v2; v2 = ROTL(v2, 32); \
> + } while(0)
> +
> +__attribute__((optimize("unroll-loops")))
Care to document why this attribute is needed? Older versions of gcc
doesn't know how to handle it properly? Faster with newer versions?
Black magic? :)
> +uint64_t siphash24(const uint8_t *data, size_t len, const uint8_t key[SIPHASH24_KEY_LEN])
> +{
> + uint64_t v0 = 0x736f6d6570736575ULL;
s/uint64_t/u64/g please.
> + uint64_t v1 = 0x646f72616e646f6dULL;
> + uint64_t v2 = 0x6c7967656e657261ULL;
> + uint64_t v3 = 0x7465646279746573ULL;
> + uint64_t b;
> + uint64_t k0 = U8TO64(key);
> + uint64_t k1 = U8TO64(key + sizeof(uint64_t));
> + uint64_t m;
> + const uint8_t *end = data + len - (len % sizeof(uint64_t));
> + const uint8_t left = len & (sizeof(uint64_t) - 1);
> + b = ((uint64_t)len) << 56;
> + v3 ^= k1;
> + v2 ^= k0;
> + v1 ^= k1;
> + v0 ^= k0;
> + for (; data != end; data += sizeof(uint64_t)) {
> + m = U8TO64(data);
> + v3 ^= m;
> + SIPROUND;
> + SIPROUND;
> + v0 ^= m;
> + }
> + switch (left) {
> + case 7: b |= ((uint64_t)data[6]) << 48;
> + case 6: b |= ((uint64_t)data[5]) << 40;
> + case 5: b |= ((uint64_t)data[4]) << 32;
> + case 4: b |= ((uint64_t)data[3]) << 24;
> + case 3: b |= ((uint64_t)data[2]) << 16;
> + case 2: b |= ((uint64_t)data[1]) << 8;
> + case 1: b |= ((uint64_t)data[0]); break;
> + case 0: break;
> + }
> + v3 ^= b;
> + SIPROUND;
> + SIPROUND;
> + v0 ^= b;
> + v2 ^= 0xff;
> + SIPROUND;
> + SIPROUND;
> + SIPROUND;
> + SIPROUND;
> + b = (v0 ^ v1) ^ (v2 ^ v3);
> + return (__force uint64_t)cpu_to_le64(b);
> +}
> +EXPORT_SYMBOL(siphash24);
EXPORT_SYMBOL_GPL()? I have to ask, sorry :)
> +
> +#ifdef DEBUG
> +static const uint8_t test_vectors[64][8] = {
> + { 0x31, 0x0e, 0x0e, 0xdd, 0x47, 0xdb, 0x6f, 0x72 },
> + { 0xfd, 0x67, 0xdc, 0x93, 0xc5, 0x39, 0xf8, 0x74 },
> + { 0x5a, 0x4f, 0xa9, 0xd9, 0x09, 0x80, 0x6c, 0x0d },
> + { 0x2d, 0x7e, 0xfb, 0xd7, 0x96, 0x66, 0x67, 0x85 },
> + { 0xb7, 0x87, 0x71, 0x27, 0xe0, 0x94, 0x27, 0xcf },
> + { 0x8d, 0xa6, 0x99, 0xcd, 0x64, 0x55, 0x76, 0x18 },
> + { 0xce, 0xe3, 0xfe, 0x58, 0x6e, 0x46, 0xc9, 0xcb },
> + { 0x37, 0xd1, 0x01, 0x8b, 0xf5, 0x00, 0x02, 0xab },
> + { 0x62, 0x24, 0x93, 0x9a, 0x79, 0xf5, 0xf5, 0x93 },
> + { 0xb0, 0xe4, 0xa9, 0x0b, 0xdf, 0x82, 0x00, 0x9e },
> + { 0xf3, 0xb9, 0xdd, 0x94, 0xc5, 0xbb, 0x5d, 0x7a },
> + { 0xa7, 0xad, 0x6b, 0x22, 0x46, 0x2f, 0xb3, 0xf4 },
> + { 0xfb, 0xe5, 0x0e, 0x86, 0xbc, 0x8f, 0x1e, 0x75 },
> + { 0x90, 0x3d, 0x84, 0xc0, 0x27, 0x56, 0xea, 0x14 },
> + { 0xee, 0xf2, 0x7a, 0x8e, 0x90, 0xca, 0x23, 0xf7 },
> + { 0xe5, 0x45, 0xbe, 0x49, 0x61, 0xca, 0x29, 0xa1 },
> + { 0xdb, 0x9b, 0xc2, 0x57, 0x7f, 0xcc, 0x2a, 0x3f },
> + { 0x94, 0x47, 0xbe, 0x2c, 0xf5, 0xe9, 0x9a, 0x69 },
> + { 0x9c, 0xd3, 0x8d, 0x96, 0xf0, 0xb3, 0xc1, 0x4b },
> + { 0xbd, 0x61, 0x79, 0xa7, 0x1d, 0xc9, 0x6d, 0xbb },
> + { 0x98, 0xee, 0xa2, 0x1a, 0xf2, 0x5c, 0xd6, 0xbe },
> + { 0xc7, 0x67, 0x3b, 0x2e, 0xb0, 0xcb, 0xf2, 0xd0 },
> + { 0x88, 0x3e, 0xa3, 0xe3, 0x95, 0x67, 0x53, 0x93 },
> + { 0xc8, 0xce, 0x5c, 0xcd, 0x8c, 0x03, 0x0c, 0xa8 },
> + { 0x94, 0xaf, 0x49, 0xf6, 0xc6, 0x50, 0xad, 0xb8 },
> + { 0xea, 0xb8, 0x85, 0x8a, 0xde, 0x92, 0xe1, 0xbc },
> + { 0xf3, 0x15, 0xbb, 0x5b, 0xb8, 0x35, 0xd8, 0x17 },
> + { 0xad, 0xcf, 0x6b, 0x07, 0x63, 0x61, 0x2e, 0x2f },
> + { 0xa5, 0xc9, 0x1d, 0xa7, 0xac, 0xaa, 0x4d, 0xde },
> + { 0x71, 0x65, 0x95, 0x87, 0x66, 0x50, 0xa2, 0xa6 },
> + { 0x28, 0xef, 0x49, 0x5c, 0x53, 0xa3, 0x87, 0xad },
> + { 0x42, 0xc3, 0x41, 0xd8, 0xfa, 0x92, 0xd8, 0x32 },
> + { 0xce, 0x7c, 0xf2, 0x72, 0x2f, 0x51, 0x27, 0x71 },
> + { 0xe3, 0x78, 0x59, 0xf9, 0x46, 0x23, 0xf3, 0xa7 },
> + { 0x38, 0x12, 0x05, 0xbb, 0x1a, 0xb0, 0xe0, 0x12 },
> + { 0xae, 0x97, 0xa1, 0x0f, 0xd4, 0x34, 0xe0, 0x15 },
> + { 0xb4, 0xa3, 0x15, 0x08, 0xbe, 0xff, 0x4d, 0x31 },
> + { 0x81, 0x39, 0x62, 0x29, 0xf0, 0x90, 0x79, 0x02 },
> + { 0x4d, 0x0c, 0xf4, 0x9e, 0xe5, 0xd4, 0xdc, 0xca },
> + { 0x5c, 0x73, 0x33, 0x6a, 0x76, 0xd8, 0xbf, 0x9a },
> + { 0xd0, 0xa7, 0x04, 0x53, 0x6b, 0xa9, 0x3e, 0x0e },
> + { 0x92, 0x59, 0x58, 0xfc, 0xd6, 0x42, 0x0c, 0xad },
> + { 0xa9, 0x15, 0xc2, 0x9b, 0xc8, 0x06, 0x73, 0x18 },
> + { 0x95, 0x2b, 0x79, 0xf3, 0xbc, 0x0a, 0xa6, 0xd4 },
> + { 0xf2, 0x1d, 0xf2, 0xe4, 0x1d, 0x45, 0x35, 0xf9 },
> + { 0x87, 0x57, 0x75, 0x19, 0x04, 0x8f, 0x53, 0xa9 },
> + { 0x10, 0xa5, 0x6c, 0xf5, 0xdf, 0xcd, 0x9a, 0xdb },
> + { 0xeb, 0x75, 0x09, 0x5c, 0xcd, 0x98, 0x6c, 0xd0 },
> + { 0x51, 0xa9, 0xcb, 0x9e, 0xcb, 0xa3, 0x12, 0xe6 },
> + { 0x96, 0xaf, 0xad, 0xfc, 0x2c, 0xe6, 0x66, 0xc7 },
> + { 0x72, 0xfe, 0x52, 0x97, 0x5a, 0x43, 0x64, 0xee },
> + { 0x5a, 0x16, 0x45, 0xb2, 0x76, 0xd5, 0x92, 0xa1 },
> + { 0xb2, 0x74, 0xcb, 0x8e, 0xbf, 0x87, 0x87, 0x0a },
> + { 0x6f, 0x9b, 0xb4, 0x20, 0x3d, 0xe7, 0xb3, 0x81 },
> + { 0xea, 0xec, 0xb2, 0xa3, 0x0b, 0x22, 0xa8, 0x7f },
> + { 0x99, 0x24, 0xa4, 0x3c, 0xc1, 0x31, 0x57, 0x24 },
> + { 0xbd, 0x83, 0x8d, 0x3a, 0xaf, 0xbf, 0x8d, 0xb7 },
> + { 0x0b, 0x1a, 0x2a, 0x32, 0x65, 0xd5, 0x1a, 0xea },
> + { 0x13, 0x50, 0x79, 0xa3, 0x23, 0x1c, 0xe6, 0x60 },
> + { 0x93, 0x2b, 0x28, 0x46, 0xe4, 0xd7, 0x06, 0x66 },
> + { 0xe1, 0x91, 0x5f, 0x5c, 0xb1, 0xec, 0xa4, 0x6c },
> + { 0xf3, 0x25, 0x96, 0x5c, 0xa1, 0x6d, 0x62, 0x9f },
> + { 0x57, 0x5f, 0xf2, 0x8e, 0x60, 0x38, 0x1b, 0xe5 },
> + { 0x72, 0x45, 0x06, 0xeb, 0x4c, 0x32, 0x8a, 0x95 }
> +};
> +
> +static int siphash24_selftest(void)
> +{
> + uint8_t in[64], k[16], i;
> + uint64_t out;
> + int ret = 0;
> +
> + for (i = 0; i < 16; ++i)
> + k[i] = i;
> +
> + for (i = 0; i < 64; ++i) {
> + in[i] = i;
> + out = siphash24(in, i, k);
> + if (memcmp(&out, test_vectors[i], 8)) {
> + printk(KERN_INFO "siphash24: self-test %u: FAIL\n", i + 1);
pr_info()?
> + ret = -1;
Pick a real error number?
> + }
> + }
> + if (!ret)
> + printk(KERN_INFO "siphash24: self-tests: pass\n");
pr_info()?
> + return ret;
> +}
> +__initcall(siphash24_selftest);
Don't we have a "do crypto/library/whatever selftests at boot" config
option that this little test could be put under? It would be great to
not have to manually add DEBUG to the build to verify this works on a
specific arch.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] siphash: add cryptographically secure hashtable function
From: Vegard Nossum @ 2016-12-10 14:17 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: LKML, kernel-hardening, linux-crypto, Rusty Russell,
Linus Torvalds, Jean-Philippe Aumasson, Daniel J . Bernstein,
linux
In-Reply-To: <20161209183659.25727-1-Jason@zx2c4.com>
On 9 December 2016 at 19:36, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> SipHash is a 64-bit keyed hash function that is actually a
> cryptographically secure PRF, like HMAC. Except SipHash is super fast,
> and is meant to be used as a hashtable keyed lookup function.
>
> SipHash isn't just some new trendy hash function. It's been around for a
> while, and there really isn't anything that comes remotely close to
> being useful in the way SipHash is. With that said, why do we need this?
>
> There are a variety of attacks known as "hashtable poisoning" in which an
> attacker forms some data such that the hash of that data will be the
> same, and then preceeds to fill up all entries of a hashbucket. This is
> a realistic and well-known denial-of-service vector.
>
> Linux developers already seem to be aware that this is an issue, and
> various places that use hash tables in, say, a network context, use a
> non-cryptographically secure function (usually jhash) and then try to
> twiddle with the key on a time basis (or in many cases just do nothing
> and hope that nobody notices). While this is an admirable attempt at
> solving the problem, it doesn't actually fix it. SipHash fixes it.
Could you give some more concrete details/examples? Here's the IPv4
hash table from include/net/inet_sock.h / net/ipv4/inet_hashtables.c:
static inline unsigned int __inet_ehashfn(const __be32 laddr,
const __u16 lport,
const __be32 faddr,
const __be16 fport,
u32 initval)
{
return jhash_3words((__force __u32) laddr,
(__force __u32) faddr,
((__u32) lport) << 16 | (__force __u32)fport,
initval);
}
static u32 inet_ehashfn(const struct net *net, const __be32 laddr,
const __u16 lport, const __be32 faddr,
const __be16 fport)
{
static u32 inet_ehash_secret __read_mostly;
net_get_random_once(&inet_ehash_secret, sizeof(inet_ehash_secret));
return __inet_ehashfn(laddr, lport, faddr, fport,
inet_ehash_secret + net_hash_mix(net));
}
There's a 32-bit secret random salt (inet_ehash_secret) which means
that in practice, inet_ehashfn() will select 1 out of 2^32 different
hash functions at random each time you boot the kernel; without
knowing which one it selected, how can a local or remote attacker can
force IPv4 connections/whatever to go into a single hash bucket?
It is not possible to obtain the secret salt directly (except by
reading from kernel memory, in which case you've lost already), nor is
it possible to obtain the result of inet_ehashfn() other than (maybe)
by a timing attack where you somehow need to detect that two
connections went into the same hash bucket and work backwards from
that to figure out how to land more connections into into the same
bucket -- but if they can do that, you've also already lost.
The same pattern is used for IPv6 hashtables and the dentry cache.
I suppose that using a hash function proven to be cryptographically
secure gives a hard guarantee (under some assumptions) that the
salt/key will give enough diversity between the (in the example above)
2^32 different hash functions that you cannot improve your chances of
guessing that two values will map to the same bucket regardless of the
salt/key. However, I am a bit doubtful that using a cryptographically
secure hash function will make much of a difference as long as the
attacker doesn't actually have any way to get the output/result of the
hash function (and given that the hash function isn't completely
trivial, of course).
I am happy to be proven wrong, but you make it sound very easy to
exploit the current situation, so I would just like to ask whether you
have a concrete way to do that?
Vegard
> There are a modicum of places in the kernel that are vulnerable to
> hashtable poisoning attacks, either via userspace vectors or network
> vectors, and there's not a reliable mechanism inside the kernel at the
> moment to fix it. The first step toward fixing these issues is actually
> getting a secure primitive into the kernel for developers to use. Then
> we can, bit by bit, port things over to it as deemed appropriate.
>
> Dozens of languages are already using this internally for their hash
> tables. Some of the BSDs already use this in their kernels. SipHash is
> a widely known high-speed solution to a widely known problem, and it's
> time we catch-up.
^ permalink raw reply
* Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Jason A. Donenfeld @ 2016-12-10 14:45 UTC (permalink / raw)
To: kernel-hardening
Cc: Andy Lutomirski, Eric Biggers, linux-crypto,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Andrew Lutomirski, Stephan Mueller
In-Reply-To: <20161210053711.GB27951@gondor.apana.org.au>
Hi Herbert,
On Sat, Dec 10, 2016 at 6:37 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> As for AEAD we never had a sync interface to begin with and I
> don't think I'm going to add one.
That's too bad to hear. I hope you'll reconsider. Modern cryptographic
design is heading more and more in the direction of using AEADs for
interesting things, and having a sync interface would be a lot easier
for implementing these protocols. In the same way many protocols need
a hash of some data, now protocols often want some particular data
encrypted with an AEAD using a particular key and nonce and AD. One
protocol that comes to mind is Noise [1].
I know that in my own [currently external to the tree] kernel code, I
just forego the use of the crypto API all together, and one of the
primary reasons for that is lack of a sync interface for AEADs. When I
eventually send this upstream, presumably everyone will want me to use
the crypto API, and having a sync AEAD interface would be personally
helpful for that. I guess I could always write the sync interface
myself, but I imagine you'd prefer having the design control etc.
Jason
[1] http://noiseprotocol.org/
^ permalink raw reply
* Re: [PATCH] siphash: add cryptographically secure hashtable function
From: George Spelvin @ 2016-12-10 15:35 UTC (permalink / raw)
To: Jason, vegard.nossum
Cc: djb, jeanphilippe.aumasson, kernel-hardening, linux-crypto,
linux-kernel, linux, rusty, torvalds
In-Reply-To: <CAOMGZ=HMTZhBOh0jTBT4cyMuK5s-D51FFUtWUWyMV7VX0U2L0w@mail.gmail.com>
> There's a 32-bit secret random salt (inet_ehash_secret) which means
> that in practice, inet_ehashfn() will select 1 out of 2^32 different
> hash functions at random each time you boot the kernel; without
> knowing which one it selected, how can a local or remote attacker can
> force IPv4 connections/whatever to go into a single hash bucket?
By figuring out the salt. The thing is, the timing of hash table lookups
*is externally visible*. If I create connections to the target, then
see which ones make responses on previous connections slightly slower,
I gain information about the salt.
I dont't know *where* in the hash table the collissions occur, but I
know *which* inputs collide, and that's enough for me to learn something.
(I need more connections than the size of the hash table, but even
with just one IP source I can use 64K ports on my end times however
many the target has open on its end.)
With enough information (google "unicity distance") I can recover the
entire salt. It's not like I care about the cryptographic strength of
the hash; simply trying all 4 billion possible seeds is pretty fast on
a 4 GHz processor.
Once that happens, I can choose a target connection whose timing I can't
observe directly and pack its hash chain without being obvious about it.
> I am happy to be proven wrong, but you make it sound very easy to
> exploit the current situation, so I would just like to ask whether you
> have a concrete way to do that?
I don't think anyone's implemented an attack on this particular hash
table yet, and the reason it hasn't been urgent is that it's just a mild
DoS attack it makes the computer noticeably slower withough disabling
it completely.
But the general style of attack is well known and has been repeatedly
demonstrated. Its practicality is not in question. The only question is
whether it's *more* practical that simpler techniques that don't depend
on any such algorithmic subtlety like brute-force flooding.
But if the history of Internet security has taught us one thing, it's
that naively hoping something won't be a problem is doomed.
The main issue is performance. IPv6 addresses are big, and although
SipHash is fast by the standard of cryptographic hashes, it's far slower
than jhash or any other non-cryptographic hash.
^ permalink raw reply
* Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
From: Andy Lutomirski @ 2016-12-10 17:48 UTC (permalink / raw)
To: Jason A. Donenfeld, Al Viro
Cc: kernel-hardening@lists.openwall.com, Eric Biggers, linux-crypto,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Andrew Lutomirski, Stephan Mueller
In-Reply-To: <CAHmME9pzT=bxuEVVGDOJkm2PaEAVjbo=8na7URy=g-1sKvv0yw@mail.gmail.com>
cc: Viro because I'm talking about iov_iter.
On Sat, Dec 10, 2016 at 6:45 AM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Hi Herbert,
>
> On Sat, Dec 10, 2016 at 6:37 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>> As for AEAD we never had a sync interface to begin with and I
>> don't think I'm going to add one.
>
> That's too bad to hear. I hope you'll reconsider. Modern cryptographic
> design is heading more and more in the direction of using AEADs for
> interesting things, and having a sync interface would be a lot easier
> for implementing these protocols. In the same way many protocols need
> a hash of some data, now protocols often want some particular data
> encrypted with an AEAD using a particular key and nonce and AD. One
> protocol that comes to mind is Noise [1].
>
I think that sync vs async has gotten conflated with
vectored-vs-nonvectored and the results are unfortunate.
There are a lot of users in the tree that are trying to do crypto on
very small pieces of data and want to have that data consist of the
concatenation of two small buffers and/or want to use primitives that
don't have "sync" interfaces. These users are stuck using async
interfaces even though using async implementations makes no sense for
them.
I'd love to see the API restructured a bit to decouple all of these
considerations. One approach might be to teach iov_iter about
scatterlists. Then, for each primitive, there could be two entry
points:
1. A simplified and lower-overhead entry. You pass it an iov_iter
(and, depending on what the operation is, an output iov_iter), it does
the crypto synchronously, and returns. Operating in-place might be
permitted for some primitives.
2. A full-featured async entry. You pass it iov_iters and it requires
that the iov_iters be backed by scatterlists in order to operate
asynchronously.
I see no reason that the decisions to use virtual vs physical
addressing or to do vectored vs non-vectored IO should be tied up with
asynchronicity.
--Andy
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] siphash: add cryptographically secure hashtable function
From: Jean-Philippe Aumasson @ 2016-12-10 18:13 UTC (permalink / raw)
To: Vegard Nossum, Jason A. Donenfeld
Cc: LKML, kernel-hardening, linux-crypto, Rusty Russell,
Linus Torvalds, Daniel J . Bernstein, linux
In-Reply-To: <CAOMGZ=HMTZhBOh0jTBT4cyMuK5s-D51FFUtWUWyMV7VX0U2L0w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5804 bytes --]
SipHash co-designer here.
SipHash is secure when it takes a secret key/seed as parameter, meaning
that its output values are unpredictable. Concretely, when SipHash produces
64-bit output values then you've a chance 1/2^64 to guess the hash value of
a given message, provided that the key/seed is kept secret. That's the
standard security definition of a pseudorandom function (PRF), which is
typically instantiated with a MAC such as HMAC-somehash.
With djb we demonstrated that this security notion is sufficient to protect
from hash-flooding attacks wherein an attacker creates many different input
values that hash to a same value and therefore may DoS the underlying data
structure.
I admit that the naming is confusing: "SipHash" is not a hash function,
strictly speaking. In crypto we only call hash function algorithms that are
unkeyed. PRFs/MACs are sometimes called keyed hash functions though.
On Sat, Dec 10, 2016 at 3:17 PM Vegard Nossum <vegard.nossum@gmail.com>
wrote:
> On 9 December 2016 at 19:36, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > SipHash is a 64-bit keyed hash function that is actually a
> > cryptographically secure PRF, like HMAC. Except SipHash is super fast,
> > and is meant to be used as a hashtable keyed lookup function.
> >
> > SipHash isn't just some new trendy hash function. It's been around for a
> > while, and there really isn't anything that comes remotely close to
> > being useful in the way SipHash is. With that said, why do we need this?
> >
> > There are a variety of attacks known as "hashtable poisoning" in which an
> > attacker forms some data such that the hash of that data will be the
> > same, and then preceeds to fill up all entries of a hashbucket. This is
> > a realistic and well-known denial-of-service vector.
> >
> > Linux developers already seem to be aware that this is an issue, and
> > various places that use hash tables in, say, a network context, use a
> > non-cryptographically secure function (usually jhash) and then try to
> > twiddle with the key on a time basis (or in many cases just do nothing
> > and hope that nobody notices). While this is an admirable attempt at
> > solving the problem, it doesn't actually fix it. SipHash fixes it.
>
> Could you give some more concrete details/examples? Here's the IPv4
> hash table from include/net/inet_sock.h / net/ipv4/inet_hashtables.c:
>
> static inline unsigned int __inet_ehashfn(const __be32 laddr,
> const __u16 lport,
> const __be32 faddr,
> const __be16 fport,
> u32 initval)
> {
> return jhash_3words((__force __u32) laddr,
> (__force __u32) faddr,
> ((__u32) lport) << 16 | (__force __u32)fport,
> initval);
> }
>
> static u32 inet_ehashfn(const struct net *net, const __be32 laddr,
> const __u16 lport, const __be32 faddr,
> const __be16 fport)
> {
> static u32 inet_ehash_secret __read_mostly;
>
> net_get_random_once(&inet_ehash_secret, sizeof(inet_ehash_secret));
>
> return __inet_ehashfn(laddr, lport, faddr, fport,
> inet_ehash_secret + net_hash_mix(net));
> }
>
> There's a 32-bit secret random salt (inet_ehash_secret) which means
> that in practice, inet_ehashfn() will select 1 out of 2^32 different
> hash functions at random each time you boot the kernel; without
> knowing which one it selected, how can a local or remote attacker can
> force IPv4 connections/whatever to go into a single hash bucket?
>
> It is not possible to obtain the secret salt directly (except by
> reading from kernel memory, in which case you've lost already), nor is
> it possible to obtain the result of inet_ehashfn() other than (maybe)
> by a timing attack where you somehow need to detect that two
> connections went into the same hash bucket and work backwards from
> that to figure out how to land more connections into into the same
> bucket -- but if they can do that, you've also already lost.
>
> The same pattern is used for IPv6 hashtables and the dentry cache.
>
> I suppose that using a hash function proven to be cryptographically
> secure gives a hard guarantee (under some assumptions) that the
> salt/key will give enough diversity between the (in the example above)
> 2^32 different hash functions that you cannot improve your chances of
> guessing that two values will map to the same bucket regardless of the
> salt/key. However, I am a bit doubtful that using a cryptographically
> secure hash function will make much of a difference as long as the
> attacker doesn't actually have any way to get the output/result of the
> hash function (and given that the hash function isn't completely
> trivial, of course).
>
> I am happy to be proven wrong, but you make it sound very easy to
> exploit the current situation, so I would just like to ask whether you
> have a concrete way to do that?
>
>
> Vegard
>
> > There are a modicum of places in the kernel that are vulnerable to
> > hashtable poisoning attacks, either via userspace vectors or network
> > vectors, and there's not a reliable mechanism inside the kernel at the
> > moment to fix it. The first step toward fixing these issues is actually
> > getting a secure primitive into the kernel for developers to use. Then
> > we can, bit by bit, port things over to it as deemed appropriate.
> >
> > Dozens of languages are already using this internally for their hash
> > tables. Some of the BSDs already use this in their kernels. SipHash is
> > a widely known high-speed solution to a widely known problem, and it's
> > time we catch-up.
>
[-- Attachment #2: Type: text/html, Size: 8549 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] crypto: brcm: Add Broadcom SPU driver DT entry.
From: kbuild test robot @ 2016-12-11 0:14 UTC (permalink / raw)
To: Rob Rice
Cc: kbuild-all-JC7UmRfGjtg, Herbert Xu, David S. Miller, Rob Herring,
Mark Rutland, linux-crypto-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ray Jui, Scott Branden,
Jon Mason, bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
Catalin Marinas, Will Deacon,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Steve Lin,
Rob Rice
In-Reply-To: <1480536453-24781-4-git-send-email-rob.rice-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]
Hi Rob,
[auto build test ERROR on cryptodev/master]
[also build test ERROR on v4.9-rc8]
[cannot apply to next-20161209]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Rob-Rice/crypto-brcm-DT-documentation-for-Broadcom-SPU-driver/20161202-010038
base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
>> ERROR: Input tree has errors, aborting (use -f to force output)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31957 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox