net: buf: linearize: Get rid of useless memset()

net_buf_linearize() used to clear the contents of output buffer,
just to fill it with data as the next step. The only effect that
would have is if less data was written to the output buffer. But
it's not reliable for a caller to rely on net_buf_linearize() for
that, instead callers should take care to handle any conditions
like that themselves. For example, a caller which wants to process
the data as zero-terminated string, must reserve a byte for it
in the output buffer explicitly (and set it to zero).

The only in-tree user which relied on clearing output buffer was
wncm14a2a.c. But either had buffer sizes calculated very precisely
to always accommodate extra trailing zero byte (without providing
code comments about this), or arguably could suffer from buffer
overruns (at least if data received from a modem was invalid and
filled up all destination buffer, leaving no space for trailing
zero).

Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>
This commit is contained in:
Paul Sokolovsky 2019-01-09 14:07:28 +02:00 committed by Kumar Gala
parent afcfa11ce0
commit c885cb533e
3 changed files with 42 additions and 16 deletions

View file

@ -562,22 +562,34 @@ static void on_cmd_atcmdecho_nosock(struct net_buf **buf, u16_t len)
static void on_cmd_atcmdinfo_manufacturer(struct net_buf **buf, u16_t len)
{
net_buf_linearize(ictx.mdm_manufacturer, sizeof(ictx.mdm_manufacturer),
*buf, 0, len);
size_t out_len;
out_len = net_buf_linearize(ictx.mdm_manufacturer,
sizeof(ictx.mdm_manufacturer) - 1,
*buf, 0, len);
ictx.mdm_manufacturer[out_len] = 0;
LOG_INF("Manufacturer: %s", ictx.mdm_manufacturer);
}
static void on_cmd_atcmdinfo_model(struct net_buf **buf, u16_t len)
{
net_buf_linearize(ictx.mdm_model, sizeof(ictx.mdm_model),
*buf, 0, len);
size_t out_len;
out_len = net_buf_linearize(ictx.mdm_model,
sizeof(ictx.mdm_model) - 1,
*buf, 0, len);
ictx.mdm_model[out_len] = 0;
LOG_INF("Model: %s", ictx.mdm_model);
}
static void on_cmd_atcmdinfo_revision(struct net_buf **buf, u16_t len)
{
net_buf_linearize(ictx.mdm_revision, sizeof(ictx.mdm_revision),
*buf, 0, len);
size_t out_len;
out_len = net_buf_linearize(ictx.mdm_revision,
sizeof(ictx.mdm_revision) - 1,
*buf, 0, len);
ictx.mdm_revision[out_len] = 0;
LOG_INF("Revision: %s", ictx.mdm_revision);
}
@ -585,6 +597,7 @@ static void on_cmd_atcmdecho_nosock_imei(struct net_buf **buf, u16_t len)
{
struct net_buf *frag = NULL;
u16_t offset;
size_t out_len;
/* make sure IMEI data is received */
if (len < MDM_IMEI_LENGTH) {
@ -607,7 +620,9 @@ static void on_cmd_atcmdecho_nosock_imei(struct net_buf **buf, u16_t len)
return;
}
net_buf_linearize(ictx.mdm_imei, sizeof(ictx.mdm_imei), *buf, 0, len);
out_len = net_buf_linearize(ictx.mdm_imei, sizeof(ictx.mdm_imei) - 1,
*buf, 0, len);
ictx.mdm_imei[out_len] = 0;
LOG_INF("IMEI: %s", ictx.mdm_imei);
}
@ -685,10 +700,12 @@ static void on_cmd_sockerror(struct net_buf **buf, u16_t len)
static void on_cmd_sockexterror(struct net_buf **buf, u16_t len)
{
char value[8];
size_t out_len;
struct wncm14a2a_socket *sock = NULL;
net_buf_linearize(value, sizeof(value), *buf, 0, len);
out_len = net_buf_linearize(value, sizeof(value) - 1, *buf, 0, len);
value[out_len] = 0;
ictx.last_error = -atoi(value);
LOG_ERR("@EXTERR:%d", ictx.last_error);
sock = socket_from_id(ictx.last_socket_id);
@ -703,8 +720,10 @@ static void on_cmd_sockexterror(struct net_buf **buf, u16_t len)
static void on_cmd_sockdial(struct net_buf **buf, u16_t len)
{
char value[8];
size_t out_len;
net_buf_linearize(value, sizeof(value), *buf, 0, len);
out_len = net_buf_linearize(value, sizeof(value) - 1, *buf, 0, len);
value[out_len] = 0;
ictx.last_error = atoi(value);
k_sem_give(&ictx.response_sem);
}
@ -729,11 +748,13 @@ static void on_cmd_sockcreat(struct net_buf **buf, u16_t len)
static void on_cmd_sockwrite(struct net_buf **buf, u16_t len)
{
char value[8];
size_t out_len;
int write_len;
struct wncm14a2a_socket *sock = NULL;
/* TODO: check against what we wanted to send */
net_buf_linearize(value, sizeof(value), *buf, 0, len);
out_len = net_buf_linearize(value, sizeof(value) - 1, *buf, 0, len);
value[out_len] = 0;
write_len = atoi(value);
if (write_len <= 0) {
return;
@ -901,12 +922,14 @@ static void on_cmd_sockread(struct net_buf **buf, u16_t len)
static void on_cmd_sockdataind(struct net_buf **buf, u16_t len)
{
int socket_id, session_status, left_bytes;
size_t out_len;
char *delim1, *delim2;
char value[sizeof("#,#,#####\r")];
char sendbuf[sizeof("AT@SOCKREAD=#,#####\r")];
struct wncm14a2a_socket *sock = NULL;
net_buf_linearize(value, sizeof(value), *buf, 0, len);
out_len = net_buf_linearize(value, sizeof(value) - 1, *buf, 0, len);
value[out_len] = 0;
/* First comma separator marks the end of socket_id */
delim1 = strchr(value, ',');
@ -961,9 +984,11 @@ static void on_cmd_sockdataind(struct net_buf **buf, u16_t len)
static void on_cmd_socknotifyev(struct net_buf **buf, u16_t len)
{
char value[40];
size_t out_len;
int p1 = 0, p2 = 0;
net_buf_linearize(value, sizeof(value), *buf, 0, len);
out_len = net_buf_linearize(value, sizeof(value) - 1, *buf, 0, len);
value[out_len] = 0;
/* walk value till 1st quote */
while (p1 < len && value[p1] != '\"') {

View file

@ -685,9 +685,6 @@ size_t net_buf_linearize(void *dst, size_t dst_len, struct net_buf *src,
frag = src;
/* clear dst */
(void)memset(dst, 0, dst_len);
/* find the right fragment to start copying from */
while (frag && offset >= frag->len) {
offset -= frag->len;

View file

@ -531,7 +531,11 @@ static inline ssize_t zsock_recv_dgram(struct net_context *ctx,
recv_len = max_len;
}
net_frag_linearize(buf, recv_len, pkt, header_len, recv_len);
/* Length passed as arguments are all based on packet data size
* and output buffer size, so return value is invariantly == recv_len,
* and we just ignore it.
*/
(void)net_frag_linearize(buf, recv_len, pkt, header_len, recv_len);
if (!(flags & ZSOCK_MSG_PEEK)) {
net_pkt_unref(pkt);