re: eCryptfs: Use skcipher and shash

re: eCryptfs: Use skcipher and shash

[Dan Carpenter]

Hello Herbert Xu,

The patch 3095e8e366b4: "eCryptfs: Use skcipher and shash" from Jan
25, 2016, leads to the following static checker warning:

	fs/ecryptfs/keystore.c:867 ecryptfs_write_tag_70_packet()
	error: potential null dereference 's'.  (kzalloc returns null)

fs/ecryptfs/keystore.c
   624  int
   625  ecryptfs_write_tag_70_packet(char *dest, size_t *remaining_bytes,
   626                               size_t *packet_size,
   627                               struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
   628                               char *filename, size_t filename_size)
   629  {
   630          struct ecryptfs_write_tag_70_packet_silly_stack *s;
   631          struct key *auth_tok_key = NULL;
   632          int rc = 0;
   633  
   634          s = kzalloc(sizeof(*s), GFP_KERNEL);
   635          if (!s) {
   636                  printk(KERN_ERR "%s: Out of memory whilst trying to kmalloc "
   637                         "[%zd] bytes of kernel memory\n", __func__, sizeof(*s));
   638                  rc = -ENOMEM;
   639                  goto out;
                        ^^^^^^^^
   640          }

[ snip ]

   862  out:
   863          if (auth_tok_key) {
   864                  up_write(&(auth_tok_key->sem));
   865                  key_put(auth_tok_key);
   866          }
   867          skcipher_request_free(s->skcipher_req);
                                      ^^^^^^^^^^^^^^^
   868          kzfree(s->hash_desc);
                       ^^^^^^^^^^^^
   869          kfree(s);
   870          return rc;
   871  }

regards,
dan carpenter

re: eCryptfs: Use skcipher and shash

[Dan Carpenter]

Hello Herbert Xu,

The patch 3095e8e366b4: "eCryptfs: Use skcipher and shash" from Jan
25, 2016, leads to the following static checker warning:

	fs/ecryptfs/keystore.c:1117 ecryptfs_parse_tag_70_packet()
	error: potential null dereference 's'.  (kzalloc returns null)

fs/ecryptfs/keystore.c
   908  int
   909  ecryptfs_parse_tag_70_packet(char **filename, size_t *filename_size,
   910                               size_t *packet_size,
   911                               struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
   912                               char *data, size_t max_packet_size)
   913  {
   914          struct ecryptfs_parse_tag_70_packet_silly_stack *s;
   915          struct key *auth_tok_key = NULL;
   916          int rc = 0;
   917  
   918          (*packet_size) = 0;
   919          (*filename_size) = 0;
   920          (*filename) = NULL;
   921          s = kzalloc(sizeof(*s), GFP_KERNEL);
   922          if (!s) {
                    ^^^
   923                  printk(KERN_ERR "%s: Out of memory whilst trying to kmalloc "
   924                         "[%zd] bytes of kernel memory\n", __func__, sizeof(*s));
   925                  rc = -ENOMEM;
   926                  goto out;
                        ^^^^^^^^
   927          }

[ snip ]

  1107  out:
  1108          if (rc) {
  1109                  (*packet_size) = 0;
  1110                  (*filename_size) = 0;
  1111                  (*filename) = NULL;
  1112          }
  1113          if (auth_tok_key) {
  1114                  up_write(&(auth_tok_key->sem));
  1115                  key_put(auth_tok_key);
  1116          }
  1117          skcipher_request_free(s->skcipher_req);
                                      ^^^^^^^^^^^^^^^
  1118          kfree(s);
  1119          return rc;
  1120  }

regards,
dan carpenter

Re: eCryptfs: Use skcipher and shash

[Herbert Xu]

On Wed, Mar 16, 2016 at 10:41:41AM +0300, Dan Carpenter wrote:
> Hello Herbert Xu,
> 
> The patch 3095e8e366b4: "eCryptfs: Use skcipher and shash" from Jan
> 25, 2016, leads to the following static checker warning:
> 
> 	fs/ecryptfs/keystore.c:1117 ecryptfs_parse_tag_70_packet()
> 	error: potential null dereference 's'.  (kzalloc returns null)

Thanks for catching this Dan.  It's probably easiest if I add
this to the crypto tree.

---8<---
eCryptfs: Fix null pointer dereference on kzalloc error path

The conversion to skcipher and shash added a couple of null pointer
dereference bugs on the kzalloc failure path.  This patch fixes them.

Fixes: 3095e8e366b4 ("eCryptfs: Use skcipher and shash")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index c5c84df..9893d15 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -635,8 +635,7 @@ ecryptfs_write_tag_70_packet(char *dest, size_t *remaining_bytes,
 	if (!s) {
 		printk(KERN_ERR "%s: Out of memory whilst trying to kmalloc "
 		       "[%zd] bytes of kernel memory\n", __func__, sizeof(*s));
-		rc = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 	(*packet_size) = 0;
 	rc = ecryptfs_find_auth_tok_for_sig(
@@ -922,8 +921,7 @@ ecryptfs_parse_tag_70_packet(char **filename, size_t *filename_size,
 	if (!s) {
 		printk(KERN_ERR "%s: Out of memory whilst trying to kmalloc "
 		       "[%zd] bytes of kernel memory\n", __func__, sizeof(*s));
-		rc = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 	if (max_packet_size < ECRYPTFS_TAG_70_MIN_METADATA_SIZE) {
 		printk(KERN_WARNING "%s: max_packet_size is [%zd]; it must be "

-- 
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