Skip to content

Commit

Permalink
Avoid expensive data copies for large packets, as for SFTP WRITE requ…
Browse files Browse the repository at this point in the history
…ests, by implementing "direct read" accessors. (#1725)
  • Loading branch information
Castaglia committed Oct 2, 2023
1 parent d94123b commit a00850b
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 128 deletions.
2 changes: 1 addition & 1 deletion contrib/mod_sftp/fxp.c
Original file line number Diff line number Diff line change
Expand Up @@ -13263,7 +13263,7 @@ static int fxp_handle_write(struct fxp_packet *fxp) {
name = sftp_msg_read_string(fxp->pool, &fxp->payload, &fxp->payload_sz);
offset = sftp_msg_read_long(fxp->pool, &fxp->payload, &fxp->payload_sz);
datalen = sftp_msg_read_int(fxp->pool, &fxp->payload, &fxp->payload_sz);
data = sftp_msg_read_data(fxp->pool, &fxp->payload, &fxp->payload_sz,
data = sftp_msg_read_data_direct(fxp->pool, &fxp->payload, &fxp->payload_sz,
datalen);

memset(cmd_arg, '\0', sizeof(cmd_arg));
Expand Down
49 changes: 42 additions & 7 deletions contrib/mod_sftp/msg.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* ProFTPD - mod_sftp message format
* Copyright (c) 2008-2022 TJ Saunders
* Copyright (c) 2008-2023 TJ Saunders
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -118,6 +118,24 @@ int sftp_msg_read_bool(pool *p, unsigned char **buf, uint32_t *buflen) {

uint32_t sftp_msg_read_data2(pool *p, unsigned char **buf,
uint32_t *buflen, size_t datalen, unsigned char **data) {
uint32_t len;

len = sftp_msg_read_data2_direct(p, buf, buflen, datalen, data);
if (len == 0) {
return 0;
}

*data = palloc(p, datalen);

memcpy(*data, *buf, datalen);
(*buf) += datalen;
(*buflen) -= datalen;

return datalen;
}

uint32_t sftp_msg_read_data2_direct(pool *p, unsigned char **buf,
uint32_t *buflen, size_t datalen, unsigned char **data) {
if (datalen == 0) {
return 0;
}
Expand All @@ -129,12 +147,7 @@ uint32_t sftp_msg_read_data2(pool *p, unsigned char **buf,
return 0;
}

*data = palloc(p, datalen);

memcpy(*data, *buf, datalen);
(*buf) += datalen;
(*buflen) -= datalen;

This comment has been minimized.

Copy link
@Castaglia

Castaglia Oct 8, 2023

Author Member

Here we deliberately avoid the memcpy(2) call, and just return a pointer to the data already in memory.


*data = *buf;
return datalen;
}

Expand All @@ -160,6 +173,28 @@ unsigned char *sftp_msg_read_data(pool *p, unsigned char **buf,
return data;
}

unsigned char *sftp_msg_read_data_direct(pool *p, unsigned char **buf,
uint32_t *buflen, size_t datalen) {
unsigned char *data = NULL;
uint32_t len;

if (datalen == 0) {
errno = EINVAL;
return NULL;
}

len = sftp_msg_read_data2_direct(p, buf, buflen, datalen, &data);
if (len == 0) {
(void) pr_log_writefile(sftp_logfd, MOD_SFTP_VERSION,
"message format error: unable to read %lu bytes of raw data "
"(buflen = %lu)", (unsigned long) datalen, (unsigned long) *buflen);
pr_log_stacktrace(sftp_logfd, MOD_SFTP_VERSION);
SFTP_DISCONNECT_CONN(SFTP_SSH2_DISCONNECT_BY_APPLICATION, NULL);
}

return data;
}

uint32_t sftp_msg_read_int2(pool *p, unsigned char **buf, uint32_t *buflen,
uint32_t *val) {

Expand Down
6 changes: 5 additions & 1 deletion contrib/mod_sftp/msg.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* ProFTPD - mod_sftp message format
* Copyright (c) 2008-2022 TJ Saunders
* Copyright (c) 2008-2023 TJ Saunders
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -30,6 +30,8 @@
unsigned char sftp_msg_read_byte(pool *, unsigned char **, uint32_t *);
int sftp_msg_read_bool(pool *, unsigned char **, uint32_t *);
unsigned char *sftp_msg_read_data(pool *, unsigned char **, uint32_t *, size_t);
unsigned char *sftp_msg_read_data_direct(pool *, unsigned char **, uint32_t *,
size_t);
#if defined(PR_USE_OPENSSL_ECC)
EC_POINT *sftp_msg_read_ecpoint(pool *, unsigned char **, uint32_t *,
const EC_GROUP *, EC_POINT *);
Expand All @@ -46,6 +48,8 @@ char *sftp_msg_read_string(pool *, unsigned char **, uint32_t *);
uint32_t sftp_msg_read_byte2(pool *, unsigned char **, uint32_t *, unsigned char *);
uint32_t sftp_msg_read_bool2(pool *, unsigned char **, uint32_t *, int *);
uint32_t sftp_msg_read_data2(pool *, unsigned char **, uint32_t *, size_t, unsigned char **);
uint32_t sftp_msg_read_data2_direct(pool *, unsigned char **, uint32_t *,
size_t, unsigned char **);
#if defined(PR_USE_OPENSSL_ECC)
uint32_t sftp_msg_read_ecpoint2(pool *, unsigned char **, uint32_t *,
const EC_GROUP *, EC_POINT **);
Expand Down
Loading

0 comments on commit a00850b

Please sign in to comment.