af_alg broken in 3.12

af_alg broken in 3.12

[Torsten Duwe]

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

Hi Herbert,

you sent a backport of 6de62f15b581f920ade22d758f4c338311c2f0d4 to be included
in the 3.12 branch (as b2a0707817d3dec83652bb460a7775613058ae), but this leaves
af_alg broken for unkeyed hash functions:

f382cd5ac26674877143fa7d9c0ea23c6640e706 (3.12 just before your commit) :

socket(PF_ALG, SOCK_SEQPACKET, 0)       = 3
bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0
accept(3, 0, NULL)                      = 4
write(4, "abc", 3)                      = 3
read(4, "\220\1P\230<\322O\260\326\226?}(\341\177r", 16) = 16

and with b2a0707817d3dec83652bb460a7775613058ae applied:

socket(PF_ALG, SOCK_SEQPACKET, 0)       = 3
bind(3, {sa_family=AF_ALG, sa_data="hash\0\0\0\0\0\0\0\0\0\0"}, 88) = 0
accept(3, 0, NULL)                      = 4
write(4, "abc", 3)                      = -1 ENOKEY (Required key not available)
read(4, 0x7ffebeba0e30, 16)             = -1 ENOKEY (Required key not available)

Mainline has meanwhile seen many fixes to this change; can you suggest an elegant
and crisp backport for these as well?

TIA,
	Torsten


[-- Attachment #2: alg.c --]
[-- Type: text/x-c++src, Size: 713 bytes --]

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <linux/if_alg.h>
#include <errno.h>

int main(void)
{
  int opfd;
  int tfmfd;
  struct sockaddr_alg sa = {
    .salg_family = AF_ALG,
    .salg_type = "hash",
    .salg_name = "md5"
  };
  char buf[20];
  int i;

  tfmfd = socket(AF_ALG, SOCK_SEQPACKET, 0);

  bind(tfmfd, (struct sockaddr *)&sa, sizeof(sa));

  opfd = accept(tfmfd, NULL, 0);

  write(opfd, "abc", 3);
  if (read(opfd, buf, 16) == -1) {
    printf("ERROR: %d\n", errno);
    exit(-1);
  }

  for (i = 0; i < 16; i++) {
    printf("%02x", (unsigned char)buf[i]);
  }
  printf("\n");

  close(opfd);
  close(tfmfd);

  return 0;
}

Re: af_alg broken in 3.12

[Torsten Duwe]

On Wed, Feb 01, 2017 at 01:13:05PM +0100, Torsten Duwe wrote:
> Hi Herbert,
> 
> you sent a backport of 6de62f15b581f920ade22d758f4c338311c2f0d4 to be included
> in the 3.12 branch (as b2a0707817d3dec83652bb460a7775613058ae), but this leaves
> af_alg broken for unkeyed hash functions:
[...]
> Mainline has meanwhile seen many fixes to this change; can you suggest an elegant
> and crisp backport for these as well?
> 

If Herbert does not have a better idea, I suggest to back out this change and fix
dynamically allocated key structures for the individual algorithms instead, for
the older branches.

Any other suggestions?

	Torsten

[PATCH] Fix af_alg in 3.12

[Torsten Duwe]

On Fri, Feb 03, 2017 at 01:05:48PM +0100, Torsten Duwe wrote:
> 
> If Herbert does not have a better idea, I suggest to back out this change and fix
> dynamically allocated key structures for the individual algorithms instead, for
> the older branches.

So, the solution IMHO is to revert

b2a0707817d3dec83652bb460a7775613058aedd
f382cd5ac26674877143fa7d9c0ea23c6640e706
c25e22ff51d3bebf579a054aecbaa98c81149c02
1e3f8a31f01e5967fcf413d72832ce41aa4efd1d
79adba68c32883c6559dc80040e97c35e208c7f1
82a0aa2c08de674191cf5e99b649af145c5ade25

that is the whole sequence from 82a0aa2c08de674191..b2a0707817d3dec836
backwards sans b9da7c51a11a2e7 which is a separate, unrelated fix.

This series aimed to fix the whole class of problems and creates and fixes
its own ABI breakage. But since that problem class contains exactly
1 element, fix instead the one broken algorithm with a dynamically
allocated member in its context that gets initialised during setkey.

Signed-off-by: Torsten Duwe <duwe@suse.de>
Cc: <stable@vger.kernel.org> # v3.0+

diff --git a/crypto/lrw.c b/crypto/lrw.c
index 6f9908a..4c9257d 100644
--- a/crypto/lrw.c
+++ b/crypto/lrw.c
@@ -150,6 +150,9 @@ static int crypt(struct blkcipher_desc *d,
 	u8 *wsrc;
 	u8 *wdst;
 
+	if (!(ctx->table.table))
+		return -ENOKEY;
+
 	err = blkcipher_walk_virt(d, w);
 	if (!(avail = w->nbytes))
 		return err;
@@ -229,6 +232,9 @@ int lrw_crypt(struct blkcipher_desc *desc, struct scatterlist *sdst,
 
 	BUG_ON(max_blks < 1);
 
+	if (!ctx->table)
+		return -ENOKEY;
+
 	blkcipher_walk_init(&walk, sdst, ssrc, nbytes);
 
 	err = blkcipher_walk_virt(desc, &walk);