Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: [PATCH v6 1/5] random: Blocking API for accessing nonblocking_pool
From: Stephan Mueller @ 2015-05-18 13:26 UTC (permalink / raw)
  To: Herbert Xu
  Cc: pebolle, andreas.steffen, tytso, sandyinchina, linux-kernel,
	linux-crypto
In-Reply-To: <7743005.ngVOaoViLi@tauon>

Am Montag, 18. Mai 2015, 15:07:10 schrieb Stephan Mueller:

Hi Herbert,

>>
>>You can simplify this further and get rid of buf/nbytes.  All
>>we need to know is whether the pool is ready.  Everything else
>>can come from private.
>
>So, the async function is now just a notification of the caller. Sounds good
>with me.
>

I am just running into an interesting problem with a missing cancel operation: 
a caller instantiates a DRBG handle and invokes the seeding operation. The 
nonblocking_pool is not initialized. Therefore, the callback is put onto the 
list for being processed later.

Now, the caller releases the DRBG handle *before* the callback is triggered.

The callback is triggered with a pointer that is invalid, but the pointer is 
non-NULL. Therefore, I am not sure how to validate the pointer in the callback 
function.

I would think that there is no other way than to add a cancel API call that 
allows removing a list entry from the wait list.


Ciao
Stephan

^ permalink raw reply

* Re: [PATCH v6 1/5] random: Blocking API for accessing nonblocking_pool
From: Theodore Ts'o @ 2015-05-18 15:02 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Herbert Xu, pebolle, andreas.steffen, sandyinchina, linux-kernel,
	linux-crypto
In-Reply-To: <2931045.EGfWxfUOa7@tauon>

On Mon, May 18, 2015 at 03:26:13PM +0200, Stephan Mueller wrote:
> 
> I am just running into an interesting problem with a missing cancel operation: 
> a caller instantiates a DRBG handle and invokes the seeding operation. The 
> nonblocking_pool is not initialized. Therefore, the callback is put onto the 
> list for being processed later.
> 
> Now, the caller releases the DRBG handle *before* the callback is triggered.
> 
> The callback is triggered with a pointer that is invalid, but the pointer is 
> non-NULL. Therefore, I am not sure how to validate the pointer in the callback 
> function.

The simplest thing to do is to put a refcount on inside the DRBG
handle structure.  The caller instantiates the DRBG handle, and
invokes the the DRBG.  The DRBG, since it is kicking off an
asynchronous operation, increments the refcount.

Both the caller and the callback function, before they exit, drop the
refcount, and if they see the refcount is zero, they free the DRBG
handle and the memory where the random seed is to be (or has been)
deposited.

This is the same pattern that the block I/O layer uses with a bio
struct.  In the BIO case, it's important since the callback function
could have been called and returned before the caller gets control
back from the bio_submit() call.  Or the struct bio may contain an
EOPNOTSUPP error, in which case there will be no callback function
dispatched.  So long as everyone handles the refcount rules, it all
works out.

Regards,

					- Ted

^ permalink raw reply

* Re: [PATCH 0/8] ARM: mvebu: Add support for RAID6 PQ offloading
From: Dan Williams @ 2015-05-18 17:06 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Vinod Koul, Gregory Clement, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, dmaengine@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-crypto, Lior Amsalem,
	Thomas Petazzoni, Herbert Xu, David S. Miller
In-Reply-To: <20150518091454.GP4004@lukather>

On Mon, May 18, 2015 at 2:14 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Dan,
>
> On Wed, May 13, 2015 at 09:00:46AM -0700, Dan Williams wrote:
>> On Wed, May 13, 2015 at 2:17 AM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > Hi Dan,
>> >
>> > On Tue, May 12, 2015 at 09:05:41AM -0700, Dan Williams wrote:
>> >> On Tue, May 12, 2015 at 8:37 AM, Maxime Ripard
>> >> <maxime.ripard@free-electrons.com> wrote:
>> >> > Hi,
>> >> >
>> >> > This serie refactors the mv_xor in order to support the latest Armada
>> >> > 38x features, including the PQ support in order to offload the RAID6
>> >> > PQ operations.
>> >> >
>> >> > Not all the PQ operations are supported by the XOR engine, so we had
>> >> > to introduce new async_tx flags in the process to identify
>> >> > un-supported operations.
>> >> >
>> >> > Please note that this is currently not usable because of a possible
>> >> > regression in the RAID stack in 4.1 that is being discussed at the
>> >> > moment here: https://lkml.org/lkml/2015/5/7/527
>> >>
>> >> This is problematic as async_tx is a wart on the dmaengine subsystem
>> >> and needs to be deprecated, I just have yet to find the time to do
>> >> that work.  It turns out it was a mistake to hide the device details
>> >> from md, it should be explicitly managing the dma channels, not
>> >> relying on a abstraction api.  The async_tx api usage of the
>> >> dma-mapping api is broken in that it relies on overlapping mappings of
>> >> the same address.  This happens to work on x86, but on arm it needs
>> >> explicit non-overlapping mappings.  I started the work to reference
>> >> count dma-mappings in 3.13, and we need to teach md to use
>> >> dmaengine_unmap_data explicitly.  Yielding dma channel management to
>> >> md also results in a more efficient implementation as we can dma_map()
>> >> the stripe cache once rather than per-io.  The  "async_tx_ack()"
>> >> disaster can also go away when md is explicitly handling channel
>> >> switching.
>> >
>> > Even though I'd be very much in favor of deprecating / removing
>> > async_tx, is it something likely to happen soon?
>>
>> Not unless someone else takes it on, I'm actively asking for help.
>>
>> > I remember discussing this with Vinod at Plumbers back in October, but
>> > haven't seen anything since then.
>>
>> Right, "help!" :)
>>
>> > If not, I think that we shouldn't really hold back patches to
>> > async_tx, even though we know than in a year from now, it's going to
>> > be gone.
>>
>> We definitely should block new usages, because they make a bad
>> situation worse.  Russell already warned that the dma_mapping api
>> abuse could lead to data corruption on ARM (speculative pre-fetching).
>> We need to mark ASYNC_TX_DMA as "depends on !ARM" or even "depends on
>> BROKEN" until we can get this resolved.
>
> I'm not sure what the issues exactly are with async_tx and ARM, but
> these patches have been tested on ARM and are working quite well.

https://lkml.org/lkml/2011/7/8/363

> What I'm doing here is merely using the existing API, I'm not making
> it worse, just using the API that is used by numerous drivers
> already. So I'm not sure this is really reasonable to ask for such a
> huge rework (with a huge potential of regressions) before merging my
> patches.

It happens.

https://lwn.net/Articles/641443/

I'm not happy about not having had the time to do this rework myself.
Linux is better off with this api deprecated.

^ permalink raw reply

* [PATCH] random: add random_initialized command line param
From: Stephan Mueller @ 2015-05-18 16:25 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-crypto, linux-kernel

Make the threshold at which the output entropy pools are considered to
be initialized configurable via a kernel command line option. The
current integer value of 128 bits is a good default value. However, some
user groups may want to use different values. For example, the SOGIS
group now requires 125 bits at least (BSI, the participant at that group
used to require 100 bits). NIST moved from 80 bits to 112 bits starting
with 2014.

It is therefore to be expected that in the future, this threshold may
increase for different user groups.

CC: Ted Tso <tytso@mit.edu>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 Documentation/kernel-parameters.txt |  7 +++++++
 drivers/char/random.c               | 26 +++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 61ab162..bc6c6f1 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2965,6 +2965,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	ramdisk_size=	[RAM] Sizes of RAM disks in kilobytes
 			See Documentation/blockdev/ramdisk.txt.
 
+	random_initialized= [KNL] Set the threshold in bits at which the
+			Linux random number generator considers an output
+			entropy pool initialized.
+			Format: <int> (must be >= 112 and <= size of output
+				       entropy pool in bits)
+			Default: 128
+
 	rcu_nocbs=	[KNL]
 			In kernels built with CONFIG_RCU_NOCB_CPU=y, set
 			the specified list of CPUs to be no-callback CPUs.
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 9cd6968..cfe4d9b 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -317,6 +317,12 @@ static int random_write_wakeup_bits = 28 * OUTPUT_POOL_WORDS;
 static int random_min_urandom_seed = 60;
 
 /*
+ * Threshold of entropy at which an entropy pool is considered to be
+ * initialized.
+ */
+static int random_initialized_threshold = 128;
+
+/*
  * Originally, we used a primitive polynomial of degree .poolwords
  * over GF(2).  The taps for various sizes are defined below.  They
  * were chosen to be evenly spaced except for the last tap, which is 1
@@ -655,7 +661,8 @@ retry:
 		goto retry;
 
 	r->entropy_total += nbits;
-	if (!r->initialized && r->entropy_total > 128) {
+	if (!r->initialized &&
+	    r->entropy_total > random_initialized_threshold) {
 		r->initialized = 1;
 		r->entropy_total = 0;
 		if (r == &nonblocking_pool) {
@@ -938,6 +945,23 @@ void add_disk_randomness(struct gendisk *disk)
 EXPORT_SYMBOL_GPL(add_disk_randomness);
 #endif
 
+/* Process kernel command-line parameter at boot time. */
+static __init int random_initalized_cmdline(char *str)
+{
+	unsigned long thresh = simple_strtoul(str, NULL, 10);
+
+	if (thresh < 112)
+		thresh = 112;
+	if (thresh > OUTPUT_POOL_WORDS * 32)
+		thresh = OUTPUT_POOL_WORDS * 32;
+	random_initialized_threshold = thresh;
+	pr_notice("random: entropy pool initialization threshold set to %d bits\n",
+		  random_initialized_threshold);
+	return 1;
+}
+
+__setup("random_initialized=", random_initalized_cmdline);
+
 /*********************************************************************
  *
  * Entropy extraction routines
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH] random: add random_initialized command line param
From: Theodore Ts'o @ 2015-05-18 18:42 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: linux-crypto, linux-kernel
In-Reply-To: <4206400.x843ypJTc1@tachyon.chronox.de>

On Mon, May 18, 2015 at 06:25:25PM +0200, Stephan Mueller wrote:
> Make the threshold at which the output entropy pools are considered to
> be initialized configurable via a kernel command line option. The
> current integer value of 128 bits is a good default value. However, some
> user groups may want to use different values. For example, the SOGIS
> group now requires 125 bits at least (BSI, the participant at that group
> used to require 100 bits). NIST moved from 80 bits to 112 bits starting
> with 2014.
> 
> It is therefore to be expected that in the future, this threshold may
> increase for different user groups.
> 
> CC: Ted Tso <tytso@mit.edu>
> Signed-off-by: Stephan Mueller <smueller@chronox.de>

How much does 125 vs 112 vs 128 bits really matter?  Is the cost of
waiting the extra 16 bits (the difference between 112 and 128 bits)
really that high?  I'm not entirely convincd that adding Yet Another
Tuning parameter is really worth it.  If we stick with 128 bits, we
will satisfy SOGIS, BSI, NIST, etc., and I would find it difficult to
believe that someone would want 132, 147, etc., bits.

	     	     	   	     	  - Ted

^ permalink raw reply

* Re: [PATCH] random: add random_initialized command line param
From: Stephan Mueller @ 2015-05-18 19:16 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: linux-crypto, linux-kernel
In-Reply-To: <20150518184209.GA2871@thunk.org>

Am Montag, 18. Mai 2015, 14:42:09 schrieb Theodore Ts'o:

Hi Theodore,

>On Mon, May 18, 2015 at 06:25:25PM +0200, Stephan Mueller wrote:
>> Make the threshold at which the output entropy pools are considered to
>> be initialized configurable via a kernel command line option. The
>> current integer value of 128 bits is a good default value. However, some
>> user groups may want to use different values. For example, the SOGIS
>> group now requires 125 bits at least (BSI, the participant at that group
>> used to require 100 bits). NIST moved from 80 bits to 112 bits starting
>> with 2014.
>> 
>> It is therefore to be expected that in the future, this threshold may
>> increase for different user groups.
>> 
>> CC: Ted Tso <tytso@mit.edu>
>> Signed-off-by: Stephan Mueller <smueller@chronox.de>
>
>How much does 125 vs 112 vs 128 bits really matter?  Is the cost of
>waiting the extra 16 bits (the difference between 112 and 128 bits)
>really that high?  I'm not entirely convincd that adding Yet Another
>Tuning parameter is really worth it.  If we stick with 128 bits, we
>will satisfy SOGIS, BSI, NIST, etc., and I would find it difficult to
>believe that someone would want 132, 147, etc., bits.

I hear more and more discussions about recommendations to use AES 256 and not 
AES 128.

These kind of recommendations will eventually also affect the entropy 
requirements for noise sources. This is my motivation for the patch: allowing 
different user groups to set the minimum bar for the nonblocking pool to 
*higher* levels (the examples for 80 to 112 bits or 100 to 125 bits shall just 
show that there are active revisions of entropy requirements).

Ciao
Stephan

^ permalink raw reply

* Re: [V4 PATCH 1/6] ACPI / scan: Parse _CCA and setup device coherency
From: Suravee Suthikulanit @ 2015-05-18 22:38 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: lenb, catalin.marinas, will.deacon, bhelgaas, thomas.lendacky,
	herbert, davem, arnd, msalter, hanjun.guo, al.stone, grant.likely,
	leo.duran, linux-arm-kernel, linux-acpi, linux-kernel,
	linaro-acpi, netdev, linux-crypto
In-Reply-To: <1880793.yBqSWvxoqF@vostro.rjw.lan>

Hi Rafael,

On 5/15/2015 6:53 PM, Rafael J. Wysocki wrote:
> On Friday, May 15, 2015 04:23:09 PM Suravee Suthikulpanit wrote:
>> [...]
>> diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
>> index 4bf7559..f6bc438 100644
>> --- a/drivers/acpi/acpi_platform.c
>> +++ b/drivers/acpi/acpi_platform.c
>> @@ -103,14 +103,18 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev)
>>   	pdevinfo.res = resources;
>>   	pdevinfo.num_res = count;
>>   	pdevinfo.fwnode = acpi_fwnode_handle(adev);
>> -	pdevinfo.dma_mask = DMA_BIT_MASK(32);
>> +	pdevinfo.dma_mask = acpi_dma_is_supported(adev) ? DMA_BIT_MASK(32) : 0;
>>   	pdev = platform_device_register_full(&pdevinfo);
>> -	if (IS_ERR(pdev))
>> +	if (IS_ERR(pdev)) {
>>   		dev_err(&adev->dev, "platform device creation failed: %ld\n",
>>   			PTR_ERR(pdev));
>> -	else
>> +	} else {
>> +		if (acpi_dma_is_supported(adev))
>> +			arch_setup_dma_ops(&pdev->dev, 0, 0, NULL,
>> +					   acpi_dma_is_coherent(adev));
>
> Shouldn't we generally do that in acpi_bind_one() for all bus types
> that don't have specific handling rather than here?

I think that would also work, and makes sense. However, I'm not sure if 
this would help in the case when we are creating PCI end-point devices, 
since the CCA is specified at the host bridge node, and there is no ACPI 
companion for the end-point devices. It seems that patch 3/6 of this 
series is still needed.


>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>> index 849b699..c56e66a 100644
>> --- a/drivers/acpi/scan.c
>> +++ b/drivers/acpi/scan.c
>> @@ -11,6 +11,7 @@
>>   #include <linux/kthread.h>
>>   #include <linux/dmi.h>
>>   #include <linux/nls.h>
>> +#include <linux/dma-mapping.h>
>>
>>   #include <asm/pgtable.h>
>>
>> @@ -2137,6 +2138,44 @@ void acpi_free_pnp_ids(struct acpi_device_pnp *pnp)
>>   	kfree(pnp->unique_id);
>>   }
>>
>> +static void acpi_init_coherency(struct acpi_device *adev)
>> +{
>> +	unsigned long long cca = 0;
>> +	acpi_status status;
>> +	struct acpi_device *parent = adev->parent;
>> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>> +
>> +	if (parent && parent->flags.cca_seen) {
>> +		/*
>> +		 * From ACPI spec, OSPM will ignore _CCA if an ancestor
>> +		 * already saw one.
>> +		 */
>> +		adev->flags.cca_seen = 1;
>> +		cca = acpi_dma_is_coherent(parent);
>
> Shouldn't the device's own _CCA take precedence?
According to the ACPI specification, the parent's _CCA take precedence.

>
>> +	} else {
>> +		status = acpi_evaluate_integer(adev->handle, "_CCA",
>> +					       NULL, &cca);
>> +		if (ACPI_SUCCESS(status)) {
>> +			adev->flags.cca_seen = 1;
>> +		} else if (!IS_ENABLED(CONFIG_ACPI_CCA_REQUIRED)) {
>> +			/*
>> +			 * If architecture does not specify that _CCA is
>> +			 * required for DMA-able devices (e.g. x86),
>> +			 * we default to _CCA=1.
>> +			 */
>> +			cca = 1;
>> +		} else {
>
> What about using acpi_handle_debug() here?
Ok I can do that.

>> [...]
>> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
>> index 8de4fa9..2a05ffb 100644
>> --- a/include/acpi/acpi_bus.h
>> +++ b/include/acpi/acpi_bus.h
>> @@ -208,7 +208,9 @@ struct acpi_device_flags {
>>   	u32 visited:1;
>>   	u32 hotplug_notify:1;
>>   	u32 is_dock_station:1;
>> -	u32 reserved:23;
>> +	u32 is_coherent:1;
>
> I'd prefer to call this 'coherent_dma'.

OK.

Thanks,

Suravee

^ permalink raw reply

* Re: [PATCH] random: add random_initialized command line param
From: Herbert Xu @ 2015-05-18 22:58 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: tytso, linux-crypto, linux-kernel
In-Reply-To: <477328243.LmeEDk1ili@tauon>

Stephan Mueller <smueller@chronox.de> wrote:
>
> I hear more and more discussions about recommendations to use AES 256 and not 
> AES 128.
> 
> These kind of recommendations will eventually also affect the entropy 
> requirements for noise sources. This is my motivation for the patch: allowing 
> different user groups to set the minimum bar for the nonblocking pool to 
> *higher* levels (the examples for 80 to 112 bits or 100 to 125 bits shall just 
> show that there are active revisions of entropy requirements).

Does anyone need to raise this from 128 today? If not then this
patch is pointless.
-- 
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: [V4 PATCH 1/6] ACPI / scan: Parse _CCA and setup device coherency
From: Rafael J. Wysocki @ 2015-05-19  0:28 UTC (permalink / raw)
  To: Suravee Suthikulanit
  Cc: lenb, catalin.marinas, will.deacon, bhelgaas, thomas.lendacky,
	herbert, davem, arnd, msalter, hanjun.guo, al.stone, grant.likely,
	leo.duran, linux-arm-kernel, linux-acpi, linux-kernel,
	linaro-acpi, netdev, linux-crypto
In-Reply-To: <555A69D9.7080508@amd.com>

On Monday, May 18, 2015 05:38:17 PM Suravee Suthikulanit wrote:
> Hi Rafael,
> 
> On 5/15/2015 6:53 PM, Rafael J. Wysocki wrote:
> > On Friday, May 15, 2015 04:23:09 PM Suravee Suthikulpanit wrote:
> >> [...]
> >> diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
> >> index 4bf7559..f6bc438 100644
> >> --- a/drivers/acpi/acpi_platform.c
> >> +++ b/drivers/acpi/acpi_platform.c
> >> @@ -103,14 +103,18 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev)
> >>   	pdevinfo.res = resources;
> >>   	pdevinfo.num_res = count;
> >>   	pdevinfo.fwnode = acpi_fwnode_handle(adev);
> >> -	pdevinfo.dma_mask = DMA_BIT_MASK(32);
> >> +	pdevinfo.dma_mask = acpi_dma_is_supported(adev) ? DMA_BIT_MASK(32) : 0;
> >>   	pdev = platform_device_register_full(&pdevinfo);
> >> -	if (IS_ERR(pdev))
> >> +	if (IS_ERR(pdev)) {
> >>   		dev_err(&adev->dev, "platform device creation failed: %ld\n",
> >>   			PTR_ERR(pdev));
> >> -	else
> >> +	} else {
> >> +		if (acpi_dma_is_supported(adev))
> >> +			arch_setup_dma_ops(&pdev->dev, 0, 0, NULL,
> >> +					   acpi_dma_is_coherent(adev));
> >
> > Shouldn't we generally do that in acpi_bind_one() for all bus types
> > that don't have specific handling rather than here?
> 
> I think that would also work, and makes sense. However, I'm not sure if 
> this would help in the case when we are creating PCI end-point devices, 
> since the CCA is specified at the host bridge node, and there is no ACPI 
> companion for the end-point devices. It seems that patch 3/6 of this 
> series is still needed.

Yes, PCI needs its own handling, but there are multiple bus types that
don't (SPI, I2C etc) in addition to the platform bus type.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Jaegeuk Kim @ 2015-05-19  5:46 UTC (permalink / raw)
  To: herbert, davem, linux-crypto, linux-ext4, linux-kernel,
	linux-f2fs-devel, ecryptfs, linux-arm-kernel
  Cc: Jaegeuk Kim

This patch adds a parameter, gfp_t, for __crypto_alloc_tfm.
Now EXT4 and F2FS use the crypto engine to encrypt a page when writing it to the
disk.
That happens during ->writepage and it needs to allocate memory with
GFP_NOFS.

Otherwise, in the f2fs case, kernel reports such the following warning.

RECLAIM_FS-ON-R at:
  [<ffffffff810e44da>] mark_held_locks+0x6a/0x90
  [<ffffffff810e516f>] lockdep_trace_alloc+0xcf/0x120
  [<ffffffff8121c573>] __kmalloc+0x53/0x3d0
  [<ffffffff81356df5>] __crypto_alloc_tfm+0x45/0x170
  [<ffffffff8135aff0>] crypto_alloc_ablkcipher+0x60/0xb0
  [<ffffffffa03e5548>] f2fs_get_crypto_ctx+0x118/0x220 [f2fs]
  [<ffffffffa03e589a>] f2fs_encrypt+0x2a/0x160 [f2fs]
  [<ffffffffa03d3eac>] do_write_data_page+0x21c/0x6f0 [f2fs]
  [<ffffffffa03d480b>] f2fs_write_data_page+0x48b/0x5c0 [f2fs]
  [<ffffffffa03cd79a>] __f2fs_writepage+0x1a/0x50 [f2fs]
  [<ffffffff811c7e44>] write_cache_pages+0x274/0x6f0
  [<ffffffffa03cf1ba>] f2fs_write_data_pages+0xea/0x3b0 [f2fs]
  [<ffffffff811c9b61>] do_writepages+0x21/0x50
  [<ffffffff812710e6>] __writeback_single_inode+0x76/0xbf0
  [<ffffffff81271f8a>] writeback_sb_inodes+0x32a/0x720
  [<ffffffff81272571>] wb_writeback+0x121/0x850
  [<ffffffff81273398>] bdi_writeback_workfn+0x148/0x980
  [<ffffffff810a74a2>] process_one_work+0x1e2/0x840
  [<ffffffff810a7c21>] worker_thread+0x121/0x470
  [<ffffffff810ae268>] kthread+0xf8/0x110
  [<ffffffff8180b9a2>] ret_from_fork+0x42/0x70

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 arch/x86/crypto/aesni-intel_glue.c      | 2 +-
 crypto/ablkcipher.c                     | 4 ++--
 crypto/aead.c                           | 2 +-
 crypto/algapi.c                         | 2 +-
 crypto/algif_skcipher.c                 | 2 +-
 crypto/api.c                            | 6 +++---
 crypto/internal.h                       | 2 +-
 crypto/tcrypt.c                         | 2 +-
 crypto/testmgr.c                        | 3 ++-
 drivers/crypto/ccp/ccp-crypto-aes-xts.c | 3 ++-
 drivers/crypto/mxs-dcp.c                | 2 +-
 drivers/crypto/picoxcell_crypto.c       | 3 ++-
 drivers/crypto/qce/ablkcipher.c         | 3 ++-
 drivers/crypto/sahara.c                 | 3 ++-
 drivers/md/dm-crypt.c                   | 3 ++-
 fs/ecryptfs/crypto.c                    | 3 ++-
 fs/ext4/crypto.c                        | 3 ++-
 fs/ext4/crypto_fname.c                  | 2 +-
 fs/f2fs/crypto.c                        | 3 ++-
 fs/f2fs/crypto_fname.c                  | 2 +-
 fs/f2fs/crypto_key.c                    | 2 +-
 include/linux/crypto.h                  | 2 +-
 22 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 112cefa..5a7fe76 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -841,7 +841,7 @@ rfc4106_set_hash_subkey(u8 *hash_subkey, const u8 *key, unsigned int key_len)
 	int ret = -EINVAL;
 	struct aesni_hash_subkey_req_data *req_data;
 
-	ctr_tfm = crypto_alloc_ablkcipher("ctr(aes)", 0, 0);
+	ctr_tfm = crypto_alloc_ablkcipher("ctr(aes)", 0, 0, GFP_KERNEL);
 	if (IS_ERR(ctr_tfm))
 		return PTR_ERR(ctr_tfm);
 
diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
index db201bca..3706e4a 100644
--- a/crypto/ablkcipher.c
+++ b/crypto/ablkcipher.c
@@ -671,7 +671,7 @@ int crypto_grab_skcipher(struct crypto_skcipher_spawn *spawn, const char *name,
 EXPORT_SYMBOL_GPL(crypto_grab_skcipher);
 
 struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
-						  u32 type, u32 mask)
+						  u32 type, u32 mask, gfp_t gfp)
 {
 	struct crypto_tfm *tfm;
 	int err;
@@ -688,7 +688,7 @@ struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, gfp);
 		if (!IS_ERR(tfm))
 			return __crypto_ablkcipher_cast(tfm);
 
diff --git a/crypto/aead.c b/crypto/aead.c
index 2222710..b220a0dd 100644
--- a/crypto/aead.c
+++ b/crypto/aead.c
@@ -542,7 +542,7 @@ struct crypto_aead *crypto_alloc_aead(const char *alg_name, u32 type, u32 mask)
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 		if (!IS_ERR(tfm))
 			return __crypto_aead_cast(tfm);
 
diff --git a/crypto/algapi.c b/crypto/algapi.c
index d2627a3..1a00274 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -660,7 +660,7 @@ struct crypto_tfm *crypto_spawn_tfm(struct crypto_spawn *spawn, u32 type,
 	if (unlikely((alg->cra_flags ^ type) & mask))
 		goto out_put_alg;
 
-	tfm = __crypto_alloc_tfm(alg, type, mask);
+	tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 	if (IS_ERR(tfm))
 		goto out_put_alg;
 
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 9450752..89730a7 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -751,7 +751,7 @@ static struct proto_ops algif_skcipher_ops = {
 
 static void *skcipher_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_ablkcipher(name, type, mask);
+	return crypto_alloc_ablkcipher(name, type, mask, GFP_KERNEL);
 }
 
 static void skcipher_release(void *private)
diff --git a/crypto/api.c b/crypto/api.c
index afe4610..887346b 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -364,14 +364,14 @@ void crypto_shoot_alg(struct crypto_alg *alg)
 EXPORT_SYMBOL_GPL(crypto_shoot_alg);
 
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
-				      u32 mask)
+				      u32 mask, gfp_t gfp)
 {
 	struct crypto_tfm *tfm = NULL;
 	unsigned int tfm_size;
 	int err = -ENOMEM;
 
 	tfm_size = sizeof(*tfm) + crypto_ctxsize(alg, type, mask);
-	tfm = kzalloc(tfm_size, GFP_KERNEL);
+	tfm = kzalloc(tfm_size, gfp);
 	if (tfm == NULL)
 		goto out_err;
 
@@ -435,7 +435,7 @@ struct crypto_tfm *crypto_alloc_base(const char *alg_name, u32 type, u32 mask)
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 		if (!IS_ERR(tfm))
 			return tfm;
 
diff --git a/crypto/internal.h b/crypto/internal.h
index bd39bfc..bd88be7 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -90,7 +90,7 @@ void crypto_remove_spawns(struct crypto_alg *alg, struct list_head *list,
 void crypto_remove_final(struct list_head *list);
 void crypto_shoot_alg(struct crypto_alg *alg);
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
-				      u32 mask);
+				      u32 mask, gfp_t);
 void *crypto_create_tfm(struct crypto_alg *alg,
 			const struct crypto_type *frontend);
 struct crypto_alg *crypto_find_alg(const char *alg_name,
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 1a28001..e6986e6 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -1097,7 +1097,7 @@ static void test_acipher_speed(const char *algo, int enc, unsigned int secs,
 
 	init_completion(&tresult.completion);
 
-	tfm = crypto_alloc_ablkcipher(algo, 0, 0);
+	tfm = crypto_alloc_ablkcipher(algo, 0, 0, GFP_KERNEL);
 
 	if (IS_ERR(tfm)) {
 		pr_err("failed to load transform for %s: %ld\n", algo,
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f9bce3d..076369f 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1563,7 +1563,8 @@ static int alg_test_skcipher(const struct alg_test_desc *desc,
 	struct crypto_ablkcipher *tfm;
 	int err = 0;
 
-	tfm = crypto_alloc_ablkcipher(driver, type | CRYPTO_ALG_INTERNAL, mask);
+	tfm = crypto_alloc_ablkcipher(driver, type | CRYPTO_ALG_INTERNAL, mask,
+								GFP_KERNEL);
 	if (IS_ERR(tfm)) {
 		printk(KERN_ERR "alg: skcipher: Failed to load transform for "
 		       "%s: %ld\n", driver, PTR_ERR(tfm));
diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 52c7395..54753ee 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -192,7 +192,8 @@ static int ccp_aes_xts_cra_init(struct crypto_tfm *tfm)
 
 	fallback_tfm = crypto_alloc_ablkcipher(crypto_tfm_alg_name(tfm), 0,
 					       CRYPTO_ALG_ASYNC |
-					       CRYPTO_ALG_NEED_FALLBACK);
+					       CRYPTO_ALG_NEED_FALLBACK,
+					       GFP_KERNEL);
 	if (IS_ERR(fallback_tfm)) {
 		pr_warn("could not load fallback driver %s\n",
 			crypto_tfm_alg_name(tfm));
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index 59ed54e..4cac3a2 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -486,7 +486,7 @@ static int mxs_dcp_aes_fallback_init(struct crypto_tfm *tfm)
 	struct dcp_async_ctx *actx = crypto_tfm_ctx(tfm);
 	struct crypto_ablkcipher *blk;
 
-	blk = crypto_alloc_ablkcipher(name, 0, flags);
+	blk = crypto_alloc_ablkcipher(name, 0, flags, GFP_KERNEL);
 	if (IS_ERR(blk))
 		return PTR_ERR(blk);
 
diff --git a/drivers/crypto/picoxcell_crypto.c b/drivers/crypto/picoxcell_crypto.c
index 5da5b98..148458e 100644
--- a/drivers/crypto/picoxcell_crypto.c
+++ b/drivers/crypto/picoxcell_crypto.c
@@ -1085,7 +1085,8 @@ static int spacc_ablk_cra_init(struct crypto_tfm *tfm)
 	ctx->generic.engine = engine;
 	if (alg->cra_flags & CRYPTO_ALG_NEED_FALLBACK) {
 		ctx->sw_cipher = crypto_alloc_ablkcipher(alg->cra_name, 0,
-				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
+				GFP_KERNEL);
 		if (IS_ERR(ctx->sw_cipher)) {
 			dev_warn(engine->dev, "failed to allocate fallback for %s\n",
 				 alg->cra_name);
diff --git a/drivers/crypto/qce/ablkcipher.c b/drivers/crypto/qce/ablkcipher.c
index ad592de..e1742d8 100644
--- a/drivers/crypto/qce/ablkcipher.c
+++ b/drivers/crypto/qce/ablkcipher.c
@@ -244,7 +244,8 @@ static int qce_ablkcipher_init(struct crypto_tfm *tfm)
 	ctx->fallback = crypto_alloc_ablkcipher(crypto_tfm_alg_name(tfm),
 						CRYPTO_ALG_TYPE_ABLKCIPHER,
 						CRYPTO_ALG_ASYNC |
-						CRYPTO_ALG_NEED_FALLBACK);
+						CRYPTO_ALG_NEED_FALLBACK,
+						GFP_KERNEL);
 	if (IS_ERR(ctx->fallback))
 		return PTR_ERR(ctx->fallback);
 
diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c
index 6be377f..50a19c1 100644
--- a/drivers/crypto/sahara.c
+++ b/drivers/crypto/sahara.c
@@ -765,7 +765,8 @@ static int sahara_aes_cra_init(struct crypto_tfm *tfm)
 	struct sahara_ctx *ctx = crypto_tfm_ctx(tfm);
 
 	ctx->fallback = crypto_alloc_ablkcipher(name, 0,
-				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
+				GFP_KERNEL);
 	if (IS_ERR(ctx->fallback)) {
 		pr_err("Error allocating fallback algo %s\n", name);
 		return PTR_ERR(ctx->fallback);
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 9eeea19..bde80c3 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1438,7 +1438,8 @@ static int crypt_alloc_tfms(struct crypt_config *cc, char *ciphermode)
 		return -ENOMEM;
 
 	for (i = 0; i < cc->tfms_count; i++) {
-		cc->tfms[i] = crypto_alloc_ablkcipher(ciphermode, 0, 0);
+		cc->tfms[i] = crypto_alloc_ablkcipher(ciphermode, 0, 0,
+							GFP_KERNEL);
 		if (IS_ERR(cc->tfms[i])) {
 			err = PTR_ERR(cc->tfms[i]);
 			crypt_free_tfms(cc);
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 97315f2..7d0d9b2 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -623,7 +623,8 @@ int ecryptfs_init_crypt_ctx(struct ecryptfs_crypt_stat *crypt_stat)
 						    crypt_stat->cipher, "cbc");
 	if (rc)
 		goto out_unlock;
-	crypt_stat->tfm = crypto_alloc_ablkcipher(full_alg_name, 0, 0);
+	crypt_stat->tfm = crypto_alloc_ablkcipher(full_alg_name, 0, 0,
+							GFP_KERNEL);
 	if (IS_ERR(crypt_stat->tfm)) {
 		rc = PTR_ERR(crypt_stat->tfm);
 		crypt_stat->tfm = NULL;
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index 8ff1527..28cbe92 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -162,7 +162,8 @@ struct ext4_crypto_ctx *ext4_get_crypto_ctx(struct inode *inode)
 		switch (key->mode) {
 		case EXT4_ENCRYPTION_MODE_AES_256_XTS:
 			ctx->tfm = crypto_ablkcipher_tfm(
-				crypto_alloc_ablkcipher("xts(aes)", 0, 0));
+				crypto_alloc_ablkcipher("xts(aes)", 0, 0,
+							GFP_NOFS));
 			break;
 		case EXT4_ENCRYPTION_MODE_AES_256_GCM:
 			/* TODO(mhalcrow): AEAD w/ gcm(aes);
diff --git a/fs/ext4/crypto_fname.c b/fs/ext4/crypto_fname.c
index fded02f..cdd07c7 100644
--- a/fs/ext4/crypto_fname.c
+++ b/fs/ext4/crypto_fname.c
@@ -372,7 +372,7 @@ struct ext4_fname_crypto_ctx *ext4_get_fname_crypto_ctx(
 		 * re-used */
 		if (ctx->ctfm == NULL) {
 			ctx->ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))",
-					0, 0);
+					0, 0, GFP_KERNEL);
 		}
 		if (IS_ERR(ctx->ctfm)) {
 			res = PTR_ERR(ctx->ctfm);
diff --git a/fs/f2fs/crypto.c b/fs/f2fs/crypto.c
index c6d1122..173727e 100644
--- a/fs/f2fs/crypto.c
+++ b/fs/f2fs/crypto.c
@@ -161,7 +161,8 @@ struct f2fs_crypto_ctx *f2fs_get_crypto_ctx(struct inode *inode)
 		switch (ci->ci_data_mode) {
 		case F2FS_ENCRYPTION_MODE_AES_256_XTS:
 			ctx->tfm = crypto_ablkcipher_tfm(
-				crypto_alloc_ablkcipher("xts(aes)", 0, 0));
+				crypto_alloc_ablkcipher("xts(aes)", 0, 0,
+							GFP_NOFS));
 			break;
 		case F2FS_ENCRYPTION_MODE_AES_256_GCM:
 			/*
diff --git a/fs/f2fs/crypto_fname.c b/fs/f2fs/crypto_fname.c
index 016c4b6..47e8e05 100644
--- a/fs/f2fs/crypto_fname.c
+++ b/fs/f2fs/crypto_fname.c
@@ -275,7 +275,7 @@ int f2fs_setup_fname_crypto(struct inode *inode)
 		return -ENOKEY;
 	}
 
-	ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))", 0, 0);
+	ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))", 0, 0, GFP_KERNEL);
 	if (!ctfm || IS_ERR(ctfm)) {
 		res = ctfm ? PTR_ERR(ctfm) : -ENOMEM;
 		printk(KERN_DEBUG "%s: error (%d) allocating crypto tfm\n",
diff --git a/fs/f2fs/crypto_key.c b/fs/f2fs/crypto_key.c
index a25b164..6be5c9f 100644
--- a/fs/f2fs/crypto_key.c
+++ b/fs/f2fs/crypto_key.c
@@ -48,7 +48,7 @@ static int f2fs_derive_key_aes(char deriving_key[F2FS_AES_128_ECB_KEY_SIZE],
 	DECLARE_F2FS_COMPLETION_RESULT(ecr);
 	struct scatterlist src_sg, dst_sg;
 	struct crypto_ablkcipher *tfm = crypto_alloc_ablkcipher("ecb(aes)", 0,
-								0);
+								0, GFP_KERNEL);
 
 	if (IS_ERR(tfm)) {
 		res = PTR_ERR(tfm);
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 10df5d2..b26ac44 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -885,7 +885,7 @@ static inline u32 crypto_skcipher_mask(u32 mask)
  *	   of an error, PTR_ERR() returns the error code.
  */
 struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
-						  u32 type, u32 mask);
+					  u32 type, u32 mask, gfp_t gfp);
 
 static inline struct crypto_tfm *crypto_ablkcipher_tfm(
 	struct crypto_ablkcipher *tfm)
-- 
2.1.1

^ permalink raw reply related

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Herbert Xu @ 2015-05-19  5:49 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel
In-Reply-To: <1432014416-39326-1-git-send-email-jaegeuk@kernel.org>

On Mon, May 18, 2015 at 10:46:56PM -0700, Jaegeuk Kim wrote:
> This patch adds a parameter, gfp_t, for __crypto_alloc_tfm.
> Now EXT4 and F2FS use the crypto engine to encrypt a page when writing it to the
> disk.
> That happens during ->writepage and it needs to allocate memory with
> GFP_NOFS.
> 
> Otherwise, in the f2fs case, kernel reports such the following warning.

Normally crypto structures should only be allocated on control
paths where sleeping or swapping is not an issue.  Why is ext4
doing crypto allocations on the data path?

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: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Jaegeuk Kim @ 2015-05-19  6:24 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel
In-Reply-To: <20150519054945.GA28060@gondor.apana.org.au>

On Tue, May 19, 2015 at 01:49:45PM +0800, Herbert Xu wrote:
> On Mon, May 18, 2015 at 10:46:56PM -0700, Jaegeuk Kim wrote:
> > This patch adds a parameter, gfp_t, for __crypto_alloc_tfm.
> > Now EXT4 and F2FS use the crypto engine to encrypt a page when writing it to the
> > disk.
> > That happens during ->writepage and it needs to allocate memory with
> > GFP_NOFS.
> > 
> > Otherwise, in the f2fs case, kernel reports such the following warning.
> 
> Normally crypto structures should only be allocated on control
> paths where sleeping or swapping is not an issue.  Why is ext4
> doing crypto allocations on the data path?
> 

Recently, Ted introduced per-file encryption feature as follows.

https://lwn.net/Articles/639427/

The call path in fs/ext4/crypto.c is:
 - writepage
  - ext4_encrypt
   - ext4_get_crypto_ctx
    - crypto_alloc_ablkcipher

AFAIK, this way can achieve to reduce memory footprint gracefully.
Just before submitting bios, fs allocates the required memory, and then end_io
will free them in pair.

Thanks,

> 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [f2fs-dev] [PATCH v2] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Jaegeuk Kim @ 2015-05-19  6:31 UTC (permalink / raw)
  To: Herbert Xu
  Cc: ecryptfs, linux-kernel, linux-f2fs-devel, linux-crypto,
	linux-ext4, davem, linux-arm-kernel
In-Reply-To: <20150519062430.GA39588@jaegeuk-mac02.hsd1.ca.comcast.net>

 Change log from v1:
  - fix missing change

-- >8 --

This patch adds a parameter, gfp_t, for __crypto_alloc_tfm.
Now EXT4 and F2FS use the crypto engine to encrypt a page when writing it to the
disk.
That happens during ->writepage and it needs to allocate memory with
GFP_NOFS.

Otherwise, in the f2fs case, kernel reports such the following warning.

RECLAIM_FS-ON-R at:
  [<ffffffff810e44da>] mark_held_locks+0x6a/0x90
  [<ffffffff810e516f>] lockdep_trace_alloc+0xcf/0x120
  [<ffffffff8121c573>] __kmalloc+0x53/0x3d0
  [<ffffffff81356df5>] __crypto_alloc_tfm+0x45/0x170
  [<ffffffff8135aff0>] crypto_alloc_ablkcipher+0x60/0xb0
  [<ffffffffa03e5548>] f2fs_get_crypto_ctx+0x118/0x220 [f2fs]
  [<ffffffffa03e589a>] f2fs_encrypt+0x2a/0x160 [f2fs]
  [<ffffffffa03d3eac>] do_write_data_page+0x21c/0x6f0 [f2fs]
  [<ffffffffa03d480b>] f2fs_write_data_page+0x48b/0x5c0 [f2fs]
  [<ffffffffa03cd79a>] __f2fs_writepage+0x1a/0x50 [f2fs]
  [<ffffffff811c7e44>] write_cache_pages+0x274/0x6f0
  [<ffffffffa03cf1ba>] f2fs_write_data_pages+0xea/0x3b0 [f2fs]
  [<ffffffff811c9b61>] do_writepages+0x21/0x50
  [<ffffffff812710e6>] __writeback_single_inode+0x76/0xbf0
  [<ffffffff81271f8a>] writeback_sb_inodes+0x32a/0x720
  [<ffffffff81272571>] wb_writeback+0x121/0x850
  [<ffffffff81273398>] bdi_writeback_workfn+0x148/0x980
  [<ffffffff810a74a2>] process_one_work+0x1e2/0x840
  [<ffffffff810a7c21>] worker_thread+0x121/0x470
  [<ffffffff810ae268>] kthread+0xf8/0x110
  [<ffffffff8180b9a2>] ret_from_fork+0x42/0x70

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 arch/x86/crypto/aesni-intel_glue.c      | 2 +-
 crypto/ablkcipher.c                     | 4 ++--
 crypto/aead.c                           | 2 +-
 crypto/algapi.c                         | 2 +-
 crypto/algif_skcipher.c                 | 2 +-
 crypto/api.c                            | 6 +++---
 crypto/internal.h                       | 2 +-
 crypto/tcrypt.c                         | 2 +-
 crypto/testmgr.c                        | 3 ++-
 drivers/crypto/ccp/ccp-crypto-aes-xts.c | 3 ++-
 drivers/crypto/mxs-dcp.c                | 2 +-
 drivers/crypto/picoxcell_crypto.c       | 3 ++-
 drivers/crypto/qce/ablkcipher.c         | 3 ++-
 drivers/crypto/sahara.c                 | 3 ++-
 drivers/md/dm-crypt.c                   | 3 ++-
 fs/ecryptfs/crypto.c                    | 3 ++-
 fs/ext4/crypto.c                        | 3 ++-
 fs/ext4/crypto_fname.c                  | 2 +-
 fs/ext4/crypto_key.c                    | 2 +-
 fs/f2fs/crypto.c                        | 3 ++-
 fs/f2fs/crypto_fname.c                  | 2 +-
 fs/f2fs/crypto_key.c                    | 2 +-
 include/linux/crypto.h                  | 2 +-
 23 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 112cefa..5a7fe76 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -841,7 +841,7 @@ rfc4106_set_hash_subkey(u8 *hash_subkey, const u8 *key, unsigned int key_len)
 	int ret = -EINVAL;
 	struct aesni_hash_subkey_req_data *req_data;
 
-	ctr_tfm = crypto_alloc_ablkcipher("ctr(aes)", 0, 0);
+	ctr_tfm = crypto_alloc_ablkcipher("ctr(aes)", 0, 0, GFP_KERNEL);
 	if (IS_ERR(ctr_tfm))
 		return PTR_ERR(ctr_tfm);
 
diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
index db201bca..3706e4a 100644
--- a/crypto/ablkcipher.c
+++ b/crypto/ablkcipher.c
@@ -671,7 +671,7 @@ int crypto_grab_skcipher(struct crypto_skcipher_spawn *spawn, const char *name,
 EXPORT_SYMBOL_GPL(crypto_grab_skcipher);
 
 struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
-						  u32 type, u32 mask)
+						  u32 type, u32 mask, gfp_t gfp)
 {
 	struct crypto_tfm *tfm;
 	int err;
@@ -688,7 +688,7 @@ struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, gfp);
 		if (!IS_ERR(tfm))
 			return __crypto_ablkcipher_cast(tfm);
 
diff --git a/crypto/aead.c b/crypto/aead.c
index 2222710..b220a0dd 100644
--- a/crypto/aead.c
+++ b/crypto/aead.c
@@ -542,7 +542,7 @@ struct crypto_aead *crypto_alloc_aead(const char *alg_name, u32 type, u32 mask)
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 		if (!IS_ERR(tfm))
 			return __crypto_aead_cast(tfm);
 
diff --git a/crypto/algapi.c b/crypto/algapi.c
index d2627a3..1a00274 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -660,7 +660,7 @@ struct crypto_tfm *crypto_spawn_tfm(struct crypto_spawn *spawn, u32 type,
 	if (unlikely((alg->cra_flags ^ type) & mask))
 		goto out_put_alg;
 
-	tfm = __crypto_alloc_tfm(alg, type, mask);
+	tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 	if (IS_ERR(tfm))
 		goto out_put_alg;
 
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 9450752..89730a7 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -751,7 +751,7 @@ static struct proto_ops algif_skcipher_ops = {
 
 static void *skcipher_bind(const char *name, u32 type, u32 mask)
 {
-	return crypto_alloc_ablkcipher(name, type, mask);
+	return crypto_alloc_ablkcipher(name, type, mask, GFP_KERNEL);
 }
 
 static void skcipher_release(void *private)
diff --git a/crypto/api.c b/crypto/api.c
index afe4610..887346b 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -364,14 +364,14 @@ void crypto_shoot_alg(struct crypto_alg *alg)
 EXPORT_SYMBOL_GPL(crypto_shoot_alg);
 
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
-				      u32 mask)
+				      u32 mask, gfp_t gfp)
 {
 	struct crypto_tfm *tfm = NULL;
 	unsigned int tfm_size;
 	int err = -ENOMEM;
 
 	tfm_size = sizeof(*tfm) + crypto_ctxsize(alg, type, mask);
-	tfm = kzalloc(tfm_size, GFP_KERNEL);
+	tfm = kzalloc(tfm_size, gfp);
 	if (tfm == NULL)
 		goto out_err;
 
@@ -435,7 +435,7 @@ struct crypto_tfm *crypto_alloc_base(const char *alg_name, u32 type, u32 mask)
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm(alg, type, mask, GFP_KERNEL);
 		if (!IS_ERR(tfm))
 			return tfm;
 
diff --git a/crypto/internal.h b/crypto/internal.h
index bd39bfc..bd88be7 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -90,7 +90,7 @@ void crypto_remove_spawns(struct crypto_alg *alg, struct list_head *list,
 void crypto_remove_final(struct list_head *list);
 void crypto_shoot_alg(struct crypto_alg *alg);
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
-				      u32 mask);
+				      u32 mask, gfp_t);
 void *crypto_create_tfm(struct crypto_alg *alg,
 			const struct crypto_type *frontend);
 struct crypto_alg *crypto_find_alg(const char *alg_name,
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 1a28001..e6986e6 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -1097,7 +1097,7 @@ static void test_acipher_speed(const char *algo, int enc, unsigned int secs,
 
 	init_completion(&tresult.completion);
 
-	tfm = crypto_alloc_ablkcipher(algo, 0, 0);
+	tfm = crypto_alloc_ablkcipher(algo, 0, 0, GFP_KERNEL);
 
 	if (IS_ERR(tfm)) {
 		pr_err("failed to load transform for %s: %ld\n", algo,
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f9bce3d..076369f 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1563,7 +1563,8 @@ static int alg_test_skcipher(const struct alg_test_desc *desc,
 	struct crypto_ablkcipher *tfm;
 	int err = 0;
 
-	tfm = crypto_alloc_ablkcipher(driver, type | CRYPTO_ALG_INTERNAL, mask);
+	tfm = crypto_alloc_ablkcipher(driver, type | CRYPTO_ALG_INTERNAL, mask,
+								GFP_KERNEL);
 	if (IS_ERR(tfm)) {
 		printk(KERN_ERR "alg: skcipher: Failed to load transform for "
 		       "%s: %ld\n", driver, PTR_ERR(tfm));
diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 52c7395..54753ee 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -192,7 +192,8 @@ static int ccp_aes_xts_cra_init(struct crypto_tfm *tfm)
 
 	fallback_tfm = crypto_alloc_ablkcipher(crypto_tfm_alg_name(tfm), 0,
 					       CRYPTO_ALG_ASYNC |
-					       CRYPTO_ALG_NEED_FALLBACK);
+					       CRYPTO_ALG_NEED_FALLBACK,
+					       GFP_KERNEL);
 	if (IS_ERR(fallback_tfm)) {
 		pr_warn("could not load fallback driver %s\n",
 			crypto_tfm_alg_name(tfm));
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index 59ed54e..4cac3a2 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -486,7 +486,7 @@ static int mxs_dcp_aes_fallback_init(struct crypto_tfm *tfm)
 	struct dcp_async_ctx *actx = crypto_tfm_ctx(tfm);
 	struct crypto_ablkcipher *blk;
 
-	blk = crypto_alloc_ablkcipher(name, 0, flags);
+	blk = crypto_alloc_ablkcipher(name, 0, flags, GFP_KERNEL);
 	if (IS_ERR(blk))
 		return PTR_ERR(blk);
 
diff --git a/drivers/crypto/picoxcell_crypto.c b/drivers/crypto/picoxcell_crypto.c
index 5da5b98..148458e 100644
--- a/drivers/crypto/picoxcell_crypto.c
+++ b/drivers/crypto/picoxcell_crypto.c
@@ -1085,7 +1085,8 @@ static int spacc_ablk_cra_init(struct crypto_tfm *tfm)
 	ctx->generic.engine = engine;
 	if (alg->cra_flags & CRYPTO_ALG_NEED_FALLBACK) {
 		ctx->sw_cipher = crypto_alloc_ablkcipher(alg->cra_name, 0,
-				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
+				GFP_KERNEL);
 		if (IS_ERR(ctx->sw_cipher)) {
 			dev_warn(engine->dev, "failed to allocate fallback for %s\n",
 				 alg->cra_name);
diff --git a/drivers/crypto/qce/ablkcipher.c b/drivers/crypto/qce/ablkcipher.c
index ad592de..e1742d8 100644
--- a/drivers/crypto/qce/ablkcipher.c
+++ b/drivers/crypto/qce/ablkcipher.c
@@ -244,7 +244,8 @@ static int qce_ablkcipher_init(struct crypto_tfm *tfm)
 	ctx->fallback = crypto_alloc_ablkcipher(crypto_tfm_alg_name(tfm),
 						CRYPTO_ALG_TYPE_ABLKCIPHER,
 						CRYPTO_ALG_ASYNC |
-						CRYPTO_ALG_NEED_FALLBACK);
+						CRYPTO_ALG_NEED_FALLBACK,
+						GFP_KERNEL);
 	if (IS_ERR(ctx->fallback))
 		return PTR_ERR(ctx->fallback);
 
diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c
index 6be377f..50a19c1 100644
--- a/drivers/crypto/sahara.c
+++ b/drivers/crypto/sahara.c
@@ -765,7 +765,8 @@ static int sahara_aes_cra_init(struct crypto_tfm *tfm)
 	struct sahara_ctx *ctx = crypto_tfm_ctx(tfm);
 
 	ctx->fallback = crypto_alloc_ablkcipher(name, 0,
-				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+				CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
+				GFP_KERNEL);
 	if (IS_ERR(ctx->fallback)) {
 		pr_err("Error allocating fallback algo %s\n", name);
 		return PTR_ERR(ctx->fallback);
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 9eeea19..bde80c3 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1438,7 +1438,8 @@ static int crypt_alloc_tfms(struct crypt_config *cc, char *ciphermode)
 		return -ENOMEM;
 
 	for (i = 0; i < cc->tfms_count; i++) {
-		cc->tfms[i] = crypto_alloc_ablkcipher(ciphermode, 0, 0);
+		cc->tfms[i] = crypto_alloc_ablkcipher(ciphermode, 0, 0,
+							GFP_KERNEL);
 		if (IS_ERR(cc->tfms[i])) {
 			err = PTR_ERR(cc->tfms[i]);
 			crypt_free_tfms(cc);
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 97315f2..7d0d9b2 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -623,7 +623,8 @@ int ecryptfs_init_crypt_ctx(struct ecryptfs_crypt_stat *crypt_stat)
 						    crypt_stat->cipher, "cbc");
 	if (rc)
 		goto out_unlock;
-	crypt_stat->tfm = crypto_alloc_ablkcipher(full_alg_name, 0, 0);
+	crypt_stat->tfm = crypto_alloc_ablkcipher(full_alg_name, 0, 0,
+							GFP_KERNEL);
 	if (IS_ERR(crypt_stat->tfm)) {
 		rc = PTR_ERR(crypt_stat->tfm);
 		crypt_stat->tfm = NULL;
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index 8ff1527..28cbe92 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -162,7 +162,8 @@ struct ext4_crypto_ctx *ext4_get_crypto_ctx(struct inode *inode)
 		switch (key->mode) {
 		case EXT4_ENCRYPTION_MODE_AES_256_XTS:
 			ctx->tfm = crypto_ablkcipher_tfm(
-				crypto_alloc_ablkcipher("xts(aes)", 0, 0));
+				crypto_alloc_ablkcipher("xts(aes)", 0, 0,
+							GFP_NOFS));
 			break;
 		case EXT4_ENCRYPTION_MODE_AES_256_GCM:
 			/* TODO(mhalcrow): AEAD w/ gcm(aes);
diff --git a/fs/ext4/crypto_fname.c b/fs/ext4/crypto_fname.c
index fded02f..cdd07c7 100644
--- a/fs/ext4/crypto_fname.c
+++ b/fs/ext4/crypto_fname.c
@@ -372,7 +372,7 @@ struct ext4_fname_crypto_ctx *ext4_get_fname_crypto_ctx(
 		 * re-used */
 		if (ctx->ctfm == NULL) {
 			ctx->ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))",
-					0, 0);
+					0, 0, GFP_KERNEL);
 		}
 		if (IS_ERR(ctx->ctfm)) {
 			res = PTR_ERR(ctx->ctfm);
diff --git a/fs/ext4/crypto_key.c b/fs/ext4/crypto_key.c
index 52170d0..9ae7058 100644
--- a/fs/ext4/crypto_key.c
+++ b/fs/ext4/crypto_key.c
@@ -45,7 +45,7 @@ static int ext4_derive_key_aes(char deriving_key[EXT4_AES_128_ECB_KEY_SIZE],
 	DECLARE_EXT4_COMPLETION_RESULT(ecr);
 	struct scatterlist src_sg, dst_sg;
 	struct crypto_ablkcipher *tfm = crypto_alloc_ablkcipher("ecb(aes)", 0,
-								0);
+								0, GFP_KERNEL);
 
 	if (IS_ERR(tfm)) {
 		res = PTR_ERR(tfm);
diff --git a/fs/f2fs/crypto.c b/fs/f2fs/crypto.c
index c6d1122..173727e 100644
--- a/fs/f2fs/crypto.c
+++ b/fs/f2fs/crypto.c
@@ -161,7 +161,8 @@ struct f2fs_crypto_ctx *f2fs_get_crypto_ctx(struct inode *inode)
 		switch (ci->ci_data_mode) {
 		case F2FS_ENCRYPTION_MODE_AES_256_XTS:
 			ctx->tfm = crypto_ablkcipher_tfm(
-				crypto_alloc_ablkcipher("xts(aes)", 0, 0));
+				crypto_alloc_ablkcipher("xts(aes)", 0, 0,
+							GFP_NOFS));
 			break;
 		case F2FS_ENCRYPTION_MODE_AES_256_GCM:
 			/*
diff --git a/fs/f2fs/crypto_fname.c b/fs/f2fs/crypto_fname.c
index 016c4b6..47e8e05 100644
--- a/fs/f2fs/crypto_fname.c
+++ b/fs/f2fs/crypto_fname.c
@@ -275,7 +275,7 @@ int f2fs_setup_fname_crypto(struct inode *inode)
 		return -ENOKEY;
 	}
 
-	ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))", 0, 0);
+	ctfm = crypto_alloc_ablkcipher("cts(cbc(aes))", 0, 0, GFP_KERNEL);
 	if (!ctfm || IS_ERR(ctfm)) {
 		res = ctfm ? PTR_ERR(ctfm) : -ENOMEM;
 		printk(KERN_DEBUG "%s: error (%d) allocating crypto tfm\n",
diff --git a/fs/f2fs/crypto_key.c b/fs/f2fs/crypto_key.c
index a25b164..6be5c9f 100644
--- a/fs/f2fs/crypto_key.c
+++ b/fs/f2fs/crypto_key.c
@@ -48,7 +48,7 @@ static int f2fs_derive_key_aes(char deriving_key[F2FS_AES_128_ECB_KEY_SIZE],
 	DECLARE_F2FS_COMPLETION_RESULT(ecr);
 	struct scatterlist src_sg, dst_sg;
 	struct crypto_ablkcipher *tfm = crypto_alloc_ablkcipher("ecb(aes)", 0,
-								0);
+								0, GFP_KERNEL);
 
 	if (IS_ERR(tfm)) {
 		res = PTR_ERR(tfm);
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 10df5d2..b26ac44 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -885,7 +885,7 @@ static inline u32 crypto_skcipher_mask(u32 mask)
  *	   of an error, PTR_ERR() returns the error code.
  */
 struct crypto_ablkcipher *crypto_alloc_ablkcipher(const char *alg_name,
-						  u32 type, u32 mask);
+					  u32 type, u32 mask, gfp_t gfp);
 
 static inline struct crypto_tfm *crypto_ablkcipher_tfm(
 	struct crypto_ablkcipher *tfm)
-- 
2.1.1

^ permalink raw reply related

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Herbert Xu @ 2015-05-19  6:32 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel
In-Reply-To: <20150519062430.GA39588@jaegeuk-mac02.hsd1.ca.comcast.net>

On Mon, May 18, 2015 at 11:24:30PM -0700, Jaegeuk Kim wrote:
>
> The call path in fs/ext4/crypto.c is:
>  - writepage
>   - ext4_encrypt
>    - ext4_get_crypto_ctx
>     - crypto_alloc_ablkcipher
> 
> AFAIK, this way can achieve to reduce memory footprint gracefully.
> Just before submitting bios, fs allocates the required memory, and then end_io
> will free them in pair.

So where does the key get generated? The crypto tfm should be
allocated when you generate the key.

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: [PATCH v2] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Herbert Xu @ 2015-05-19  6:32 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: ecryptfs, linux-kernel, linux-f2fs-devel, linux-crypto,
	linux-ext4, davem, linux-arm-kernel
In-Reply-To: <20150519063142.GA39834@jaegeuk-mac02.hsd1.ca.comcast.net>

On Mon, May 18, 2015 at 11:31:42PM -0700, Jaegeuk Kim wrote:
>  Change log from v1:
>   - fix missing change

Please do not resend your patch until you have addressed my
questions.
-- 
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

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y

^ permalink raw reply

* Re: [PATCH v6 1/5] random: Blocking API for accessing nonblocking_pool
From: Stephan Mueller @ 2015-05-19  5:58 UTC (permalink / raw)
  To: Theodore Ts'o, Herbert Xu
  Cc: pebolle, andreas.steffen, sandyinchina, linux-kernel,
	linux-crypto
In-Reply-To: <20150518150234.GA4180@thunk.org>

Am Montag, 18. Mai 2015, 11:02:34 schrieb Theodore Ts'o:

Hi Theodore, Herbert,
> 
> The simplest thing to do is to put a refcount on inside the DRBG
> handle structure.  The caller instantiates the DRBG handle, and
> invokes the the DRBG.  The DRBG, since it is kicking off an
> asynchronous operation, increments the refcount.

That is a good idea. After experimenting with the refcount, I see that kernel 
crypto API release function of crypto_destroy_tfm unconditionally destroys the 
crypto handle by freeing it.

So, if a caller releases the DRBG handle, the DRBG code cannot prevent the 
destruction of its context with a refcount.

Herbert, do you have any ideas?

-- 
Ciao
Stephan

^ permalink raw reply

* Re: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Jaegeuk Kim @ 2015-05-19  6:58 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel
In-Reply-To: <20150519063211.GA28347@gondor.apana.org.au>

On Tue, May 19, 2015 at 02:32:11PM +0800, Herbert Xu wrote:
> On Mon, May 18, 2015 at 11:24:30PM -0700, Jaegeuk Kim wrote:
> >
> > The call path in fs/ext4/crypto.c is:
> >  - writepage
> >   - ext4_encrypt
> >    - ext4_get_crypto_ctx
> >     - crypto_alloc_ablkcipher
> > 
> > AFAIK, this way can achieve to reduce memory footprint gracefully.
> > Just before submitting bios, fs allocates the required memory, and then end_io
> > will free them in pair.
> 
> So where does the key get generated? The crypto tfm should be
> allocated when you generate the key.

In fs/ext4/crypto.c,

 - writepage
   - ext4_encrypt
     - ext4_get_crypto_ctx
       - crypto_alloc_ablkcipher
     - ext4_page_crypto
       - crypto_ablkcipher_setkey
       - ablkcipher_request_alloc(GFP_NOFS)
       - ablkcipher_request_set_crypt(PAGE_CACHE_SIZE)
       - crypto_ablkcipher_encrypt

 - end_io
   - crypto_free_tfm

Thanks,

> 
> 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: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Herbert Xu @ 2015-05-19  6:59 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel
In-Reply-To: <20150519065812.GA40012@jaegeuk-mac02.hsd1.ca.comcast.net>

On Mon, May 18, 2015 at 11:58:12PM -0700, Jaegeuk Kim wrote:
>
> > So where does the key get generated? The crypto tfm should be
> > allocated when you generate the key.
> 
> In fs/ext4/crypto.c,
> 
>  - writepage
>    - ext4_encrypt
>      - ext4_get_crypto_ctx
>        - crypto_alloc_ablkcipher
>      - ext4_page_crypto
>        - crypto_ablkcipher_setkey
>        - ablkcipher_request_alloc(GFP_NOFS)
>        - ablkcipher_request_set_crypt(PAGE_CACHE_SIZE)
>        - crypto_ablkcipher_encrypt
> 
>  - end_io
>    - crypto_free_tfm

No that's where you set the key, not where you generate the key.
-- 
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: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Jaegeuk Kim @ 2015-05-19  7:13 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel
In-Reply-To: <20150519065929.GA28610@gondor.apana.org.au>

On Tue, May 19, 2015 at 02:59:29PM +0800, Herbert Xu wrote:
> On Mon, May 18, 2015 at 11:58:12PM -0700, Jaegeuk Kim wrote:
> >
> > > So where does the key get generated? The crypto tfm should be
> > > allocated when you generate the key.
> > 
> > In fs/ext4/crypto.c,
> > 
> >  - writepage
> >    - ext4_encrypt
> >      - ext4_get_crypto_ctx
> >        - crypto_alloc_ablkcipher
> >      - ext4_page_crypto
> >        - crypto_ablkcipher_setkey
> >        - ablkcipher_request_alloc(GFP_NOFS)
> >        - ablkcipher_request_set_crypt(PAGE_CACHE_SIZE)
> >        - crypto_ablkcipher_encrypt
> > 
> >  - end_io
> >    - crypto_free_tfm
> 
> No that's where you set the key, not where you generate the key.

The key generation is done by ext4_generate_encryption_key in
fs/ext4/crypto_key.c.
And, ext4_file_mmap and ext4_file_open trigger it.

> -- 
> 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: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Herbert Xu @ 2015-05-19  7:15 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel
In-Reply-To: <20150519071317.GB40012@jaegeuk-mac02.hsd1.ca.comcast.net>

On Tue, May 19, 2015 at 12:13:17AM -0700, Jaegeuk Kim wrote:
>
> The key generation is done by ext4_generate_encryption_key in
> fs/ext4/crypto_key.c.
> And, ext4_file_mmap and ext4_file_open trigger it.

Well that's where you should be doing crypto_alloc_ablkcipher
and crypto_ablkcipher_setkey.

The whole point of a crypto tfm is to represent a key so any time
you get one you should create a crypto tfm.  Carrying around a raw
key is just wrong.

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: [PATCH v6 1/5] random: Blocking API for accessing nonblocking_pool
From: Herbert Xu @ 2015-05-19  7:22 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Theodore Ts'o, pebolle, andreas.steffen, sandyinchina,
	linux-kernel, linux-crypto
In-Reply-To: <6511355.kQJsTdtLzf@tachyon.chronox.de>

On Tue, May 19, 2015 at 07:58:25AM +0200, Stephan Mueller wrote:
> 
> Herbert, do you have any ideas?

On the /dev/random side,

1) Add a struct module argument in addition to func/data.
2) Grab module ref count when func/data is registered.
3) Drop module ref count after func returns.

On the drbg side,

1) Allocate data pointer before func/data registration, it should
contain a flag indicating whether drbg is still alive.
2) In cra_exit, zap the flag in allocated data.
3) In func immediately return if flag indicates drbg is dead.
4) Free allocated data pointer when func is done.

Obviously you need to add some locking so that func doesn't race
against cra_exit or any other drbg paths that it intersects.

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: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Jaegeuk Kim @ 2015-05-19  7:27 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel
In-Reply-To: <20150519071521.GA28862@gondor.apana.org.au>

On Tue, May 19, 2015 at 03:15:21PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 12:13:17AM -0700, Jaegeuk Kim wrote:
> >
> > The key generation is done by ext4_generate_encryption_key in
> > fs/ext4/crypto_key.c.
> > And, ext4_file_mmap and ext4_file_open trigger it.
> 
> Well that's where you should be doing crypto_alloc_ablkcipher
> and crypto_ablkcipher_setkey.
> 
> The whole point of a crypto tfm is to represent a key so any time
> you get one you should create a crypto tfm.  Carrying around a raw
> key is just wrong.

So, IMHO, it can consume memory too much, since tfm should be allocated for
every inodes and be alive until inode eviction.
Apart from giving GFP_NOFS, do you mean that it is a wrong approach?

Thanks,

> 
> 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: [PATCH] crypto: allow to assign gfp_t for __crypto_alloc_tfm
From: Herbert Xu @ 2015-05-19  7:30 UTC (permalink / raw)
  To: Jaegeuk Kim
  Cc: davem, linux-crypto, linux-ext4, linux-kernel, linux-f2fs-devel,
	ecryptfs, linux-arm-kernel
In-Reply-To: <20150519072740.GA40281@jaegeuk-mac02.hsd1.ca.comcast.net>

On Tue, May 19, 2015 at 12:27:40AM -0700, Jaegeuk Kim wrote:
>
> So, IMHO, it can consume memory too much, since tfm should be allocated for
> every inodes and be alive until inode eviction.

Are you sure this is a real problem? Have you actually looked at
how much memory it consumes?

> Apart from giving GFP_NOFS, do you mean that it is a wrong approach?

Allocating the tfm and setting a key on the data path is not
acceptable.

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: [PATCH v6 1/5] random: Blocking API for accessing nonblocking_pool
From: Stephan Mueller @ 2015-05-19  7:35 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Theodore Ts'o, pebolle, andreas.steffen, sandyinchina,
	linux-kernel, linux-crypto
In-Reply-To: <20150519072227.GA28837@gondor.apana.org.au>

Am Dienstag, 19. Mai 2015, 15:22:27 schrieb Herbert Xu:

Hi Herbert,

> On Tue, May 19, 2015 at 07:58:25AM +0200, Stephan Mueller wrote:
> > Herbert, do you have any ideas?
> 
> On the /dev/random side,
> 
> 1) Add a struct module argument in addition to func/data.
> 2) Grab module ref count when func/data is registered.
> 3) Drop module ref count after func returns.
> 
> On the drbg side,
> 
> 1) Allocate data pointer before func/data registration, it should
> contain a flag indicating whether drbg is still alive.
> 2) In cra_exit, zap the flag in allocated data.
> 3) In func immediately return if flag indicates drbg is dead.
> 4) Free allocated data pointer when func is done.
> 
> Obviously you need to add some locking so that func doesn't race
> against cra_exit or any other drbg paths that it intersects.

Thank you for the hints. I will follow your guidance.

Just for my edification: why is this (rather complex sounding) approach 
preferred over a simple cancel API? Other async APIs (e.g. the AIO syscalls 
with io_cancel) have such cancel operations.

Such cancel function would be as simple as:

void get_blocking_random_bytes_cancel(void *private)
{
        struct random_work *rw, *tmp;

        mutex_lock(&random_wait_list_mutex);
        list_for_each_entry_safe(rw, tmp, &random_wait_list, list) {
                if (private == rw->rw_private) {
                        list_del(&rw->list);
                        kfree(rw);
                        break;
                }
        }
        mutex_unlock(&random_wait_list_mutex);
}

-- 
Ciao
Stephan

^ permalink raw reply

* Re: [PATCH v6 1/5] random: Blocking API for accessing nonblocking_pool
From: Herbert Xu @ 2015-05-19  7:51 UTC (permalink / raw)
  To: Stephan Mueller
  Cc: Theodore Ts'o, pebolle, andreas.steffen, sandyinchina,
	linux-kernel, linux-crypto
In-Reply-To: <2404915.vbuKFfjVR8@tachyon.chronox.de>

On Tue, May 19, 2015 at 09:35:15AM +0200, Stephan Mueller wrote:
> 
> Thank you for the hints. I will follow your guidance.
> 
> Just for my edification: why is this (rather complex sounding) approach 
> preferred over a simple cancel API? Other async APIs (e.g. the AIO syscalls 
> with io_cancel) have such cancel operations.
> 
> Such cancel function would be as simple as:

You're right.  The cancel function is indeed simpler.  I can
certainly live with that.

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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox