Re: is size correct in ecryptfs_parse_packet_length()

[Tyler Hicks <tyhicks@canonical.com>]

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

Hi Dan - Thanks for taking a look at the code!

On 2014-10-21 15:04:35, Dan Carpenter wrote:
> fs/ecryptfs/keystore.c +93
>     85  /**
>     86   * ecryptfs_parse_packet_length
>     87   * @data: Pointer to memory containing length at offset
>     88   * @size: This function writes the decoded size to this memory
>     89   *        address; zero on error
>     90   * @length_size: The number of bytes occupied by the encoded length
>     91   *
>     92   * Returns zero on success; non-zero on error
>     93   */
>     94  int ecryptfs_parse_packet_length(unsigned char *data, size_t *size,
>     95                                   size_t *length_size)
>     96  {
>     97          int rc = 0;
>     98  
>     99          (*length_size) = 0;
>    100          (*size) = 0;
>    101          if (data[0] < 192) {
>    102                  /* One-byte length */
>    103                  (*size) = (unsigned char)data[0];
>    104                  (*length_size) = 1;
>    105          } else if (data[0] < 224) {
>    106                  /* Two-byte length */
>    107                  (*size) = (((unsigned char)(data[0]) - 192) * 256);
>                                     ^^^^^^^^^^^^^^^
>    108                  (*size) += ((unsigned char)(data[1]) + 192);
>                                     ^^^^^^^^^^^^^^^
> These casts are no-ops because they are "data" is an unsigned char
> pointer already.  Then the value is type promoted to int, we subtract
> 192 giving a negative number and we multiply by 256 giving a slightly
> larger negative then we save it as a very large positive.

Subtracting 192 from data[0] should never result in a negative number.
We know that data[0] is greater than or equal to 192 (and less than 224)
because the previous conditional was false.

> 
> I don't know this well enough to say what the intent was.

This method of specifying packet sizes came from RFC2440 "OpenPGP
Message Format" (https://tools.ietf.org/html/rfc2440#section-4.2.2).

Tyler

> 
>    109                  (*length_size) = 2;
>    110          } else if (data[0] == 255) {
>    111                  /* If support is added, adjust ECRYPTFS_MAX_PKT_LEN_SIZE */
>    112                  ecryptfs_printk(KERN_ERR, "Five-byte packet length not "
>    113                                  "supported\n");
>    114                  rc = -EINVAL;
>    115                  goto out;
>    116          } else {
>    117                  ecryptfs_printk(KERN_ERR, "Error parsing packet length\n");
>    118                  rc = -EINVAL;
>    119                  goto out;
>    120          }
>    121  out:
>    122          return rc;
>    123  }
> 
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]