Skip to content

Commit

Permalink
drbd: Convert from ahash to shash
Browse files Browse the repository at this point in the history 
In preparing to remove all stack VLA usage from the kernel[1], this
removes the discouraged use of AHASH_REQUEST_ON_STACK in favor of
the smaller SHASH_DESC_ON_STACK by converting from ahash-wrapped-shash
to direct shash. By removing a layer of indirection this both improves
performance and reduces stack usage. The stack allocation will be made
a fixed size in a later patch to the crypto subsystem.

The bulk of the lines in this change are simple s/ahash/shash/, but the
main logic differences are in drbd_csum_ee() and drbd_csum_bio(), which
externalizes the page walking with k(un)map_atomic() instead of using
scattergather.

[1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com

Acked-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
  • Loading branch information
Kees Cook authored and Jens Axboe committed Sep 6, 2018
1 parent ca16eb3 commit 3d0e637
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 88 deletions.
13 changes: 7 additions & 6 deletions drivers/block/drbd/drbd_int.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -724,10 +724,10 @@ struct drbd_connection {
struct list_head transfer_log; /* all requests not yet fully processed */

struct crypto_shash *cram_hmac_tfm;
struct crypto_ahash *integrity_tfm; /* checksums we compute, updates protected by connection->data->mutex */
struct crypto_ahash *peer_integrity_tfm; /* checksums we verify, only accessed from receiver thread */
struct crypto_ahash *csums_tfm;
struct crypto_ahash *verify_tfm;
struct crypto_shash *integrity_tfm; /* checksums we compute, updates protected by connection->data->mutex */
struct crypto_shash *peer_integrity_tfm; /* checksums we verify, only accessed from receiver thread */
struct crypto_shash *csums_tfm;
struct crypto_shash *verify_tfm;
void *int_dig_in;
void *int_dig_vv;

Expand Down Expand Up @@ -1531,8 +1531,9 @@ static inline void ov_out_of_sync_print(struct drbd_device *device)
}


extern void drbd_csum_bio(struct crypto_ahash *, struct bio *, void *);
extern void drbd_csum_ee(struct crypto_ahash *, struct drbd_peer_request *, void *);
extern void drbd_csum_bio(struct crypto_shash *, struct bio *, void *);
extern void drbd_csum_ee(struct crypto_shash *, struct drbd_peer_request *,
void *);
/* worker callbacks */
extern int w_e_end_data_req(struct drbd_work *, int);
extern int w_e_end_rsdata_req(struct drbd_work *, int);
Expand Down
14 changes: 7 additions & 7 deletions drivers/block/drbd/drbd_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1377,7 +1377,7 @@ void drbd_send_ack_dp(struct drbd_peer_device *peer_device, enum drbd_packet cmd
struct p_data *dp, int data_size)
{
if (peer_device->connection->peer_integrity_tfm)
data_size -= crypto_ahash_digestsize(peer_device->connection->peer_integrity_tfm);
data_size -= crypto_shash_digestsize(peer_device->connection->peer_integrity_tfm);
_drbd_send_ack(peer_device, cmd, dp->sector, cpu_to_be32(data_size),
dp->block_id);
}
Expand Down Expand Up @@ -1690,7 +1690,7 @@ int drbd_send_dblock(struct drbd_peer_device *peer_device, struct drbd_request *
sock = &peer_device->connection->data;
p = drbd_prepare_command(peer_device, sock);
digest_size = peer_device->connection->integrity_tfm ?
crypto_ahash_digestsize(peer_device->connection->integrity_tfm) : 0;
crypto_shash_digestsize(peer_device->connection->integrity_tfm) : 0;

if (!p)
return -EIO;
Expand Down Expand Up @@ -1796,7 +1796,7 @@ int drbd_send_block(struct drbd_peer_device *peer_device, enum drbd_packet cmd,
p = drbd_prepare_command(peer_device, sock);

digest_size = peer_device->connection->integrity_tfm ?
crypto_ahash_digestsize(peer_device->connection->integrity_tfm) : 0;
crypto_shash_digestsize(peer_device->connection->integrity_tfm) : 0;

if (!p)
return -EIO;
Expand Down Expand Up @@ -2557,11 +2557,11 @@ void conn_free_crypto(struct drbd_connection *connection)
{
drbd_free_sock(connection);

crypto_free_ahash(connection->csums_tfm);
crypto_free_ahash(connection->verify_tfm);
crypto_free_shash(connection->csums_tfm);
crypto_free_shash(connection->verify_tfm);
crypto_free_shash(connection->cram_hmac_tfm);
crypto_free_ahash(connection->integrity_tfm);
crypto_free_ahash(connection->peer_integrity_tfm);
crypto_free_shash(connection->integrity_tfm);
crypto_free_shash(connection->peer_integrity_tfm);
kfree(connection->int_dig_in);
kfree(connection->int_dig_vv);

Expand Down
39 changes: 12 additions & 27 deletions drivers/block/drbd/drbd_nl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2303,10 +2303,10 @@ check_net_options(struct drbd_connection *connection, struct net_conf *new_net_c
}

struct crypto {
struct crypto_ahash *verify_tfm;
struct crypto_ahash *csums_tfm;
struct crypto_shash *verify_tfm;
struct crypto_shash *csums_tfm;
struct crypto_shash *cram_hmac_tfm;
struct crypto_ahash *integrity_tfm;
struct crypto_shash *integrity_tfm;
};

static int
Expand All @@ -2324,36 +2324,21 @@ alloc_shash(struct crypto_shash **tfm, char *tfm_name, int err_alg)
return NO_ERROR;
}

static int
alloc_ahash(struct crypto_ahash **tfm, char *tfm_name, int err_alg)
{
if (!tfm_name[0])
return NO_ERROR;

*tfm = crypto_alloc_ahash(tfm_name, 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(*tfm)) {
*tfm = NULL;
return err_alg;
}

return NO_ERROR;
}

static enum drbd_ret_code
alloc_crypto(struct crypto *crypto, struct net_conf *new_net_conf)
{
char hmac_name[CRYPTO_MAX_ALG_NAME];
enum drbd_ret_code rv;

rv = alloc_ahash(&crypto->csums_tfm, new_net_conf->csums_alg,
rv = alloc_shash(&crypto->csums_tfm, new_net_conf->csums_alg,
ERR_CSUMS_ALG);
if (rv != NO_ERROR)
return rv;
rv = alloc_ahash(&crypto->verify_tfm, new_net_conf->verify_alg,
rv = alloc_shash(&crypto->verify_tfm, new_net_conf->verify_alg,
ERR_VERIFY_ALG);
if (rv != NO_ERROR)
return rv;
rv = alloc_ahash(&crypto->integrity_tfm, new_net_conf->integrity_alg,
rv = alloc_shash(&crypto->integrity_tfm, new_net_conf->integrity_alg,
ERR_INTEGRITY_ALG);
if (rv != NO_ERROR)
return rv;
Expand All @@ -2371,9 +2356,9 @@ alloc_crypto(struct crypto *crypto, struct net_conf *new_net_conf)
static void free_crypto(struct crypto *crypto)
{
crypto_free_shash(crypto->cram_hmac_tfm);
crypto_free_ahash(crypto->integrity_tfm);
crypto_free_ahash(crypto->csums_tfm);
crypto_free_ahash(crypto->verify_tfm);
crypto_free_shash(crypto->integrity_tfm);
crypto_free_shash(crypto->csums_tfm);
crypto_free_shash(crypto->verify_tfm);
}

int drbd_adm_net_opts(struct sk_buff *skb, struct genl_info *info)
Expand Down Expand Up @@ -2450,17 +2435,17 @@ int drbd_adm_net_opts(struct sk_buff *skb, struct genl_info *info)
rcu_assign_pointer(connection->net_conf, new_net_conf);

if (!rsr) {
crypto_free_ahash(connection->csums_tfm);
crypto_free_shash(connection->csums_tfm);
connection->csums_tfm = crypto.csums_tfm;
crypto.csums_tfm = NULL;
}
if (!ovr) {
crypto_free_ahash(connection->verify_tfm);
crypto_free_shash(connection->verify_tfm);
connection->verify_tfm = crypto.verify_tfm;
crypto.verify_tfm = NULL;
}

crypto_free_ahash(connection->integrity_tfm);
crypto_free_shash(connection->integrity_tfm);
connection->integrity_tfm = crypto.integrity_tfm;
if (connection->cstate >= C_WF_REPORT_PARAMS && connection->agreed_pro_version >= 100)
/* Do this without trying to take connection->data.mutex again. */
Expand Down
35 changes: 18 additions & 17 deletions drivers/block/drbd/drbd_receiver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1732,7 +1732,7 @@ static int receive_Barrier(struct drbd_connection *connection, struct packet_inf
}

/* quick wrapper in case payload size != request_size (write same) */
static void drbd_csum_ee_size(struct crypto_ahash *h,
static void drbd_csum_ee_size(struct crypto_shash *h,
struct drbd_peer_request *r, void *d,
unsigned int payload_size)
{
Expand Down Expand Up @@ -1769,7 +1769,7 @@ read_in_block(struct drbd_peer_device *peer_device, u64 id, sector_t sector,

digest_size = 0;
if (!trim && peer_device->connection->peer_integrity_tfm) {
digest_size = crypto_ahash_digestsize(peer_device->connection->peer_integrity_tfm);
digest_size = crypto_shash_digestsize(peer_device->connection->peer_integrity_tfm);
/*
* FIXME: Receive the incoming digest into the receive buffer
* here, together with its struct p_data?
Expand Down Expand Up @@ -1905,7 +1905,7 @@ static int recv_dless_read(struct drbd_peer_device *peer_device, struct drbd_req

digest_size = 0;
if (peer_device->connection->peer_integrity_tfm) {
digest_size = crypto_ahash_digestsize(peer_device->connection->peer_integrity_tfm);
digest_size = crypto_shash_digestsize(peer_device->connection->peer_integrity_tfm);
err = drbd_recv_all_warn(peer_device->connection, dig_in, digest_size);
if (err)
return err;
Expand Down Expand Up @@ -3542,7 +3542,7 @@ static int receive_protocol(struct drbd_connection *connection, struct packet_in
int p_proto, p_discard_my_data, p_two_primaries, cf;
struct net_conf *nc, *old_net_conf, *new_net_conf = NULL;
char integrity_alg[SHARED_SECRET_MAX] = "";
struct crypto_ahash *peer_integrity_tfm = NULL;
struct crypto_shash *peer_integrity_tfm = NULL;
void *int_dig_in = NULL, *int_dig_vv = NULL;

p_proto = be32_to_cpu(p->protocol);
Expand Down Expand Up @@ -3623,15 +3623,15 @@ static int receive_protocol(struct drbd_connection *connection, struct packet_in
* change.
*/

peer_integrity_tfm = crypto_alloc_ahash(integrity_alg, 0, CRYPTO_ALG_ASYNC);
peer_integrity_tfm = crypto_alloc_shash(integrity_alg, 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(peer_integrity_tfm)) {
peer_integrity_tfm = NULL;
drbd_err(connection, "peer data-integrity-alg %s not supported\n",
integrity_alg);
goto disconnect;
}

hash_size = crypto_ahash_digestsize(peer_integrity_tfm);
hash_size = crypto_shash_digestsize(peer_integrity_tfm);
int_dig_in = kmalloc(hash_size, GFP_KERNEL);
int_dig_vv = kmalloc(hash_size, GFP_KERNEL);
if (!(int_dig_in && int_dig_vv)) {
Expand Down Expand Up @@ -3661,7 +3661,7 @@ static int receive_protocol(struct drbd_connection *connection, struct packet_in
mutex_unlock(&connection->resource->conf_update);
mutex_unlock(&connection->data.mutex);

crypto_free_ahash(connection->peer_integrity_tfm);
crypto_free_shash(connection->peer_integrity_tfm);
kfree(connection->int_dig_in);
kfree(connection->int_dig_vv);
connection->peer_integrity_tfm = peer_integrity_tfm;
Expand All @@ -3679,7 +3679,7 @@ static int receive_protocol(struct drbd_connection *connection, struct packet_in
disconnect_rcu_unlock:
rcu_read_unlock();
disconnect:
crypto_free_ahash(peer_integrity_tfm);
crypto_free_shash(peer_integrity_tfm);
kfree(int_dig_in);
kfree(int_dig_vv);
conn_request_state(connection, NS(conn, C_DISCONNECTING), CS_HARD);
Expand All @@ -3691,15 +3691,16 @@ static int receive_protocol(struct drbd_connection *connection, struct packet_in
* return: NULL (alg name was "")
* ERR_PTR(error) if something goes wrong
* or the crypto hash ptr, if it worked out ok. */
static struct crypto_ahash *drbd_crypto_alloc_digest_safe(const struct drbd_device *device,
static struct crypto_shash *drbd_crypto_alloc_digest_safe(
const struct drbd_device *device,
const char *alg, const char *name)
{
struct crypto_ahash *tfm;
struct crypto_shash *tfm;

if (!alg[0])
return NULL;

tfm = crypto_alloc_ahash(alg, 0, CRYPTO_ALG_ASYNC);
tfm = crypto_alloc_shash(alg, 0, 0);
if (IS_ERR(tfm)) {
drbd_err(device, "Can not allocate \"%s\" as %s (reason: %ld)\n",
alg, name, PTR_ERR(tfm));
Expand Down Expand Up @@ -3752,8 +3753,8 @@ static int receive_SyncParam(struct drbd_connection *connection, struct packet_i
struct drbd_device *device;
struct p_rs_param_95 *p;
unsigned int header_size, data_size, exp_max_sz;
struct crypto_ahash *verify_tfm = NULL;
struct crypto_ahash *csums_tfm = NULL;
struct crypto_shash *verify_tfm = NULL;
struct crypto_shash *csums_tfm = NULL;
struct net_conf *old_net_conf, *new_net_conf = NULL;
struct disk_conf *old_disk_conf = NULL, *new_disk_conf = NULL;
const int apv = connection->agreed_pro_version;
Expand Down Expand Up @@ -3900,14 +3901,14 @@ static int receive_SyncParam(struct drbd_connection *connection, struct packet_i
if (verify_tfm) {
strcpy(new_net_conf->verify_alg, p->verify_alg);
new_net_conf->verify_alg_len = strlen(p->verify_alg) + 1;
crypto_free_ahash(peer_device->connection->verify_tfm);
crypto_free_shash(peer_device->connection->verify_tfm);
peer_device->connection->verify_tfm = verify_tfm;
drbd_info(device, "using verify-alg: \"%s\"\n", p->verify_alg);
}
if (csums_tfm) {
strcpy(new_net_conf->csums_alg, p->csums_alg);
new_net_conf->csums_alg_len = strlen(p->csums_alg) + 1;
crypto_free_ahash(peer_device->connection->csums_tfm);
crypto_free_shash(peer_device->connection->csums_tfm);
peer_device->connection->csums_tfm = csums_tfm;
drbd_info(device, "using csums-alg: \"%s\"\n", p->csums_alg);
}
Expand Down Expand Up @@ -3951,9 +3952,9 @@ static int receive_SyncParam(struct drbd_connection *connection, struct packet_i
mutex_unlock(&connection->resource->conf_update);
/* just for completeness: actually not needed,
* as this is not reached if csums_tfm was ok. */
crypto_free_ahash(csums_tfm);
crypto_free_shash(csums_tfm);
/* but free the verify_tfm again, if csums_tfm did not work out */
crypto_free_ahash(verify_tfm);
crypto_free_shash(verify_tfm);
conn_request_state(peer_device->connection, NS(conn, C_DISCONNECTING), CS_HARD);
return -EIO;
}
Expand Down
Loading

0 comments on commit 3d0e637

Please sign in to comment.