diff mbox

[1/2] crypto: export crypto_alg_list and rwsem

Message ID 1439432654-23322-1-git-send-email-iamjoonsoo.kim@lge.com (mailing list archive)
State Rejected
Delegated to: Herbert Xu
Headers show

Commit Message

Joonsoo Kim Aug. 13, 2015, 2:24 a.m. UTC
Until now, zram uses compression algorithm through direct call
to core algorithm function, but, it has drawback that we need to add
compression algorithm manually to zram. If we don't do that, we cannot
utilize various compression algorithms in the system. To improve this
situation, zram will be changed to use crypto subsystem in following
patch. There is one problem with this change. Zram has a interface
that what compression algorithm it can support. Although crypto subsystem
has /proc interface to search all of crypto algorithm, but, there is
no way to get just compression algorithm in cryto subsystem. To implement
it on zram-side, crypto_alg_list and rwsem should be exported so
this patch do it.

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
 crypto/internal.h      | 2 --
 include/linux/crypto.h | 4 ++++
 2 files changed, 4 insertions(+), 2 deletions(-)

Comments

Herbert Xu Aug. 13, 2015, 3:19 a.m. UTC | #1
On Thu, Aug 13, 2015 at 11:24:13AM +0900, Joonsoo Kim wrote:
> Until now, zram uses compression algorithm through direct call
> to core algorithm function, but, it has drawback that we need to add
> compression algorithm manually to zram. If we don't do that, we cannot
> utilize various compression algorithms in the system. To improve this
> situation, zram will be changed to use crypto subsystem in following
> patch. There is one problem with this change. Zram has a interface
> that what compression algorithm it can support. Although crypto subsystem
> has /proc interface to search all of crypto algorithm, but, there is
> no way to get just compression algorithm in cryto subsystem. To implement
> it on zram-side, crypto_alg_list and rwsem should be exported so
> this patch do it.
> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Nack.  We already have a netlink interface that can be used to
query algorithms and the type information is present in the report.
The interface is crypto_user and should be used instead of exporting
the raw list.

Cheers,
Joonsoo Kim Aug. 13, 2015, 6:37 a.m. UTC | #2
On Thu, Aug 13, 2015 at 11:19:54AM +0800, Herbert Xu wrote:
> On Thu, Aug 13, 2015 at 11:24:13AM +0900, Joonsoo Kim wrote:
> > Until now, zram uses compression algorithm through direct call
> > to core algorithm function, but, it has drawback that we need to add
> > compression algorithm manually to zram. If we don't do that, we cannot
> > utilize various compression algorithms in the system. To improve this
> > situation, zram will be changed to use crypto subsystem in following
> > patch. There is one problem with this change. Zram has a interface
> > that what compression algorithm it can support. Although crypto subsystem
> > has /proc interface to search all of crypto algorithm, but, there is
> > no way to get just compression algorithm in cryto subsystem. To implement
> > it on zram-side, crypto_alg_list and rwsem should be exported so
> > this patch do it.
> > 
> > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> 
> Nack.  We already have a netlink interface that can be used to
> query algorithms and the type information is present in the report.
> The interface is crypto_user and should be used instead of exporting
> the raw list.

Oh... I see.

Is there any way to access netlink interface and get the output from
kernel-side? I'd like to show information through
"/sys/block/zramX/comp_algorithm", because some user program can be
broken if we change output of userspace exposed interface.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu Aug. 13, 2015, 6:38 a.m. UTC | #3
On Thu, Aug 13, 2015 at 03:37:55PM +0900, Joonsoo Kim wrote:
>
> Is there any way to access netlink interface and get the output from
> kernel-side? I'd like to show information through
> "/sys/block/zramX/comp_algorithm", because some user program can be
> broken if we change output of userspace exposed interface.

You could simply deprecate that interface but keep it for existing
algorithms.  Any new algorithms can then be queried through the
crypto_user interface.

The other option is to have a defined list of algorithms which is
independent of the crypto API.  For example, have a look at what
IPsec does in net/xfrm/xfrm_algo.c.  IPsec needs its own list
because they come with annotations.

Cheers,
Herbert Xu Aug. 13, 2015, 7:29 a.m. UTC | #4
On Thu, Aug 13, 2015 at 04:30:31PM +0900, Joonsoo Kim wrote:
>
> How about introducing new functions to search supported algorithm in
> kernel-side? As crypto API is used in more places, this interface
> would be requested more. Defined list weaken the advantage of strong
> point of generic crypto API.

You're only getting the list so you can immediately reexport
it to user-space, with no added value whatsoever.  As I said
we already have an interface for that so you should use it.

If more genuine uses come up then I will reconsider.

Cheers,
Joonsoo Kim Aug. 13, 2015, 7:30 a.m. UTC | #5
On Thu, Aug 13, 2015 at 02:38:23PM +0800, Herbert Xu wrote:
> On Thu, Aug 13, 2015 at 03:37:55PM +0900, Joonsoo Kim wrote:
> >
> > Is there any way to access netlink interface and get the output from
> > kernel-side? I'd like to show information through
> > "/sys/block/zramX/comp_algorithm", because some user program can be
> > broken if we change output of userspace exposed interface.
> 
> You could simply deprecate that interface but keep it for existing
> algorithms.  Any new algorithms can then be queried through the
> crypto_user interface.
> 
> The other option is to have a defined list of algorithms which is
> independent of the crypto API.  For example, have a look at what
> IPsec does in net/xfrm/xfrm_algo.c.  IPsec needs its own list
> because they come with annotations.

Hmm... they looks suboptimal.

How about introducing new functions to search supported algorithm in
kernel-side? As crypto API is used in more places, this interface
would be requested more. Defined list weaken the advantage of strong
point of generic crypto API.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joonsoo Kim Aug. 13, 2015, 7:43 a.m. UTC | #6
On Thu, Aug 13, 2015 at 03:29:18PM +0800, Herbert Xu wrote:
> On Thu, Aug 13, 2015 at 04:30:31PM +0900, Joonsoo Kim wrote:
> >
> > How about introducing new functions to search supported algorithm in
> > kernel-side? As crypto API is used in more places, this interface
> > would be requested more. Defined list weaken the advantage of strong
> > point of generic crypto API.
> 
> You're only getting the list so you can immediately reexport
> it to user-space, with no added value whatsoever.  As I said
> we already have an interface for that so you should use it.
> 
> If more genuine uses come up then I will reconsider.

Okay. I will think this issue more and come back with better
solution.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/crypto/internal.h b/crypto/internal.h
index 00e42a3..806f17a 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -45,8 +45,6 @@  struct crypto_larval {
 	u32 mask;
 };
 
-extern struct list_head crypto_alg_list;
-extern struct rw_semaphore crypto_alg_sem;
 extern struct blocking_notifier_head crypto_chain;
 
 #ifdef CONFIG_PROC_FS
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 81ef938..ab39f4b 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -24,6 +24,10 @@ 
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/uaccess.h>
+#include <linux/rwsem.h>
+
+extern struct list_head crypto_alg_list;
+extern struct rw_semaphore crypto_alg_sem;
 
 /*
  * Autoloaded crypto modules should only use a prefixed name to avoid allowing