CHROMIUM: verity: don't call page_address on unmapped highmem pages

drivers/md/dm-bht.c

  /*
  * Copyright (C) 2010 The Chromium OS Authors <chromium-os-dev@chromium.org>
  *
  * Device-Mapper block hash tree interface.
  * See Documentation/device-mapper/dm-bht.txt for details.
  *
  * This file is released under the GPL.
  */
 
 #include <asm/atomic.h>
@@ skipping 77 matching lines @@
         dm_bht_bin_to_hex(computed, computed_hex, bht->digest_size);
         DMERR_LIMIT("%s != %s", given_hex, computed_hex);
 }
 
 /* Used for turning verifiers into computers */
 typedef int (*dm_bht_compare_cb)(struct dm_bht *, u8 *, u8 *);
 
 /**
  * dm_bht_compute_hash: hashes a page of data
  */
-static int dm_bht_compute_hash(struct dm_bht *bht, const void *block,
-                               u8 *digest)
+static int dm_bht_compute_hash(struct dm_bht *bht, struct page *pg,
+                               unsigned int offset, u8 *digest)
 {
         struct hash_desc *hash_desc = &bht->hash_desc[smp_processor_id()];
         struct scatterlist sg;
 
-        /* TODO(msb): Once we supporting block_size < PAGE_SIZE, change this to:
-         *            offset_into_page + length < page_size
-         * For now just check that block is page-aligned.
-         */
-        /*
-         * TODO(msb): Re-enable once user-space code is modified to use
-         *            aligned buffers.
-         * BUG_ON(!IS_ALIGNED((uintptr_t)block, PAGE_SIZE));
-         */
-
         sg_init_table(&sg, 1);
-        sg_set_buf(&sg, block, PAGE_SIZE);
+        sg_set_page(&sg, pg, PAGE_SIZE, 0);

[Will Drewry, 2011/04/13 20:19:30]

should 0 be the new offset argument? Then the comment could be /* TODO:
BUG_ON(offset != 0); */?

         /* Note, this is synchronous. */
         if (crypto_hash_init(hash_desc)) {
                 DMCRIT("failed to reinitialize crypto hash (proc:%d)",
                         smp_processor_id());
                 return -EINVAL;
         }
         if (crypto_hash_digest(hash_desc, &sg, PAGE_SIZE, digest)) {
                 DMCRIT("crypto_hash_digest failed");
                 return -EINVAL;
         }
@@ skipping 413 matching lines @@
                 }
                 /* Current entry is claimed for allocation and loading */
                 node_page = (struct page *) mempool_alloc(bht->entry_pool,
                                                           GFP_NOIO);
                 if (!node_page) {
                         DMCRIT("failed to allocate memory for "
                                "entry->nodes from pool");
                         return -ENOMEM;
                 }
                 /* dm-bht guarantees page-aligned memory for callbacks. */
                 entry->nodes = page_address(node_page);

[Will Drewry, 2011/04/13 20:19:30]

Will this call be a problem?

[Paul T, 2011/04/13 20:47:55]

This must assume the page is mapped. If it is not, this will be a problem. I
thought the crypto code was doing the mapping/unmapping calls.

[Mandeep Singh Baines, 2011/04/13 21:11:16]

I clarified the description. page_address is OK on a highmem page as long as its
mapped.

This call is OK because we didn't explicitly ask for __GFP_HIGHMEM. By default,
you do not get HIGHMEM pages.

                 /* Let the caller know that not all the data is yet available */
                 status |= DM_BHT_ENTRY_REQUESTED;
                 /* Issue the read callback */
                 /* TODO(wad) error check callback here too */
                 DMDEBUG("dm_bht_maybe_read_entries(d=%u,ei=%u,count=%u): "
                         "reading %lu",
                         depth, index, count,
                         (unsigned long)(entry - level->entries));
                 bht->read_cb(ctx,   /* external context */
                              current_sector,  /* starting sector */
@@ skipping 86 matching lines @@
         }
         /* Could do a cmpxchg, but this should be safe. */
         atomic_set(&bht->root_state, DM_BHT_ENTRY_VERIFIED);
         return 0;
 }
 
 /* dm_bht_verify_path
  * Verifies the path. Returns 0 on ok.
  */
 static int dm_bht_verify_path(struct dm_bht *bht, unsigned int block_index,
-                              const void *block)
+                              struct page *pg, unsigned int offset)
 {
         unsigned int depth = bht->depth;
         struct dm_bht_entry *entry;
         int state;
 
         do {
                 u8 digest[DM_BHT_MAX_DIGEST_SIZE];
                 u8 *node;
 
                 /* Need to check that the hash of the current block is accurate
                  * in its parent.
                  */
                 entry = dm_bht_get_entry(bht, depth - 1, block_index);
                 state = atomic_read(&entry->state);
                 /* This call is only safe if all nodes along the path
                  * are already populated (i.e. READY) via dm_bht_populate.
                  */
                 BUG_ON(state < DM_BHT_ENTRY_READY);
                 node = dm_bht_get_node(bht, entry, depth, block_index);
 
-                if (dm_bht_compute_hash(bht, block, digest) ||
+                if (dm_bht_compute_hash(bht, pg, offset, digest) ||
                     dm_bht_compare_hash(bht, digest, node))
                         goto mismatch;
 
                 /* Keep the containing block of hashes to be verified in the
                  * next pass.
                  */
-                block = entry->nodes;
+                pg = virt_to_page(entry->nodes);

[Will Drewry, 2011/04/13 20:19:30]

Hrm should we yet-again convert nodes back to a struct page, or is virt_to_page
safewhile page_address isn;t?

[Mandeep Singh Baines, 2011/04/13 21:11:16]

The virt_to_page here is safe because we did not use __GFP_HIGHMEM in our
allocation.

+                offset = 0;
         } while (--depth > 0 && state != DM_BHT_ENTRY_VERIFIED);
 
         /* Mark path to leaf as verified. */
         for (depth++; depth < bht->depth; depth++) {
                 entry = dm_bht_get_entry(bht, depth, block_index);
                 /* At this point, entry can only be in VERIFIED or READY state.
                  * So it is safe to use atomic_set instead of atomic_cmpxchg.
                  */
                 atomic_set(&entry->state, DM_BHT_ENTRY_VERIFIED);
         }
@@ skipping 74 matching lines @@
                 atomic_set(&entry->state, DM_BHT_ENTRY_READY);
         } else if (state <= DM_BHT_ENTRY_ERROR) {
                 DMCRIT("leaf entry for block %u is invalid",
                       block_index);
                 return state;
         } else if (state == DM_BHT_ENTRY_PENDING) {
                 DMERR("leaf data is pending for block %u", block_index);
                 return 1;
         }
 
-        dm_bht_compute_hash(bht, block_data,
+        dm_bht_compute_hash(bht, virt_to_page(block_data), 0,

[Olof Johansson, 2011/04/13 21:11:57]

Currently block_data is guaranteed to be page aligned, but it'd still be nice to
do the math on offset instead of pass 0, to avoid surprises later -- especially
if/when blocksize smaller than pagesize is used.

                             dm_bht_node(bht, entry, node_index));
         return 0;
 }
 EXPORT_SYMBOL(dm_bht_store_block);
 
 /**
  * dm_bht_zeroread_callback - read callback which always returns 0s
  * @ctx:        ignored
  * @start:      ignored
  * @data:       buffer to write 0s to
@@ skipping 49 matching lines @@
                         goto out;
                 }
 
                 for (i = 0; i < level->count; i++, entry++) {
                         unsigned int count = bht->node_count;
                         if (i == (level->count - 1))
                                 count = child_level->count % bht->node_count;
                         if (count == 0)
                                 count = bht->node_count;
                         for (j = 0; j < count; j++, child++) {
-                                u8 *block = child->nodes;
+                                struct page *pg = virt_to_page(child->nodes);
                                 u8 *digest = dm_bht_node(bht, entry, j);
 
-                                r = dm_bht_compute_hash(bht, block, digest);
+                                r = dm_bht_compute_hash(bht, pg, 0, digest);

[Olof Johansson, 2011/04/13 21:11:57]

Same here, should probably compute offset?

                                 if (r) {
                                         DMERR("Failed to update (d=%u,i=%u)",
                                               depth, i);
                                         goto out;
                                 }
                         }
                 }
         }
         /* Don't forget the root digest! */
         DMDEBUG("Calling verify_root with update_hash");
@@ skipping 156 matching lines @@
  * @bht:        pointer to a dm_bht_create()d bht
  * @block_index:specific block data is expected from
  * @block:      virtual address of the block data in memory
  *              (must be aligned to block size)
  *
  * Returns 0 on success, 1 on missing data, and a negative error
  * code on verification failure. All supporting functions called
  * should return similarly.
  */
 int dm_bht_verify_block(struct dm_bht *bht, unsigned int block_index,
-                        const void *block)
+                        struct page *pg, unsigned int offset)
 {
         int r = 0;
 
         /* Make sure that the root has been verified */
         if (atomic_read(&bht->root_state) != DM_BHT_ENTRY_VERIFIED) {
                 r = dm_bht_verify_root(bht, dm_bht_compare_hash);
                 if (r) {
                         DMERR_LIMIT("Failed to verify root: %d", r);
                         goto out;
                 }
         }
 
         /* Now check levels in between */
-        r = dm_bht_verify_path(bht, block_index, block);
+        r = dm_bht_verify_path(bht, block_index, pg, offset);
         if (r)
                 DMERR_LIMIT("Failed to verify block: %u (%d)", block_index, r);
 
 out:
         return r;
 }
 EXPORT_SYMBOL(dm_bht_verify_block);
 
 /**
  * dm_bht_destroy - cleans up all memory used by @bht
@@ skipping 127 matching lines @@
         if (!bht->root_digest) {
                 DMERR("no root digest exists to export");
                 if (available > 0)
                         *hexdigest = 0;
                 return -1;
         }
         dm_bht_bin_to_hex(bht->root_digest, hexdigest, bht->digest_size);
         return 0;
 }
 EXPORT_SYMBOL(dm_bht_root_hexdigest);