Skip to content

Commit c885cb5

Browse files
pfalcongalak
authored andcommitted
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 <[email protected]>
1 parent afcfa11 commit c885cb5

File tree

3 files changed

+42
-16
lines changed

3 files changed

+42
-16
lines changed

drivers/modem/wncm14a2a.c

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -562,29 +562,42 @@ static void on_cmd_atcmdecho_nosock(struct net_buf **buf, u16_t len)
562562

563563
static void on_cmd_atcmdinfo_manufacturer(struct net_buf **buf, u16_t len)
564564
{
565-
net_buf_linearize(ictx.mdm_manufacturer, sizeof(ictx.mdm_manufacturer),
566-
*buf, 0, len);
565+
size_t out_len;
566+
567+
out_len = net_buf_linearize(ictx.mdm_manufacturer,
568+
sizeof(ictx.mdm_manufacturer) - 1,
569+
*buf, 0, len);
570+
ictx.mdm_manufacturer[out_len] = 0;
567571
LOG_INF("Manufacturer: %s", ictx.mdm_manufacturer);
568572
}
569573

570574
static void on_cmd_atcmdinfo_model(struct net_buf **buf, u16_t len)
571575
{
572-
net_buf_linearize(ictx.mdm_model, sizeof(ictx.mdm_model),
573-
*buf, 0, len);
576+
size_t out_len;
577+
578+
out_len = net_buf_linearize(ictx.mdm_model,
579+
sizeof(ictx.mdm_model) - 1,
580+
*buf, 0, len);
581+
ictx.mdm_model[out_len] = 0;
574582
LOG_INF("Model: %s", ictx.mdm_model);
575583
}
576584

577585
static void on_cmd_atcmdinfo_revision(struct net_buf **buf, u16_t len)
578586
{
579-
net_buf_linearize(ictx.mdm_revision, sizeof(ictx.mdm_revision),
580-
*buf, 0, len);
587+
size_t out_len;
588+
589+
out_len = net_buf_linearize(ictx.mdm_revision,
590+
sizeof(ictx.mdm_revision) - 1,
591+
*buf, 0, len);
592+
ictx.mdm_revision[out_len] = 0;
581593
LOG_INF("Revision: %s", ictx.mdm_revision);
582594
}
583595

584596
static void on_cmd_atcmdecho_nosock_imei(struct net_buf **buf, u16_t len)
585597
{
586598
struct net_buf *frag = NULL;
587599
u16_t offset;
600+
size_t out_len;
588601

589602
/* make sure IMEI data is received */
590603
if (len < MDM_IMEI_LENGTH) {
@@ -607,7 +620,9 @@ static void on_cmd_atcmdecho_nosock_imei(struct net_buf **buf, u16_t len)
607620
return;
608621
}
609622

610-
net_buf_linearize(ictx.mdm_imei, sizeof(ictx.mdm_imei), *buf, 0, len);
623+
out_len = net_buf_linearize(ictx.mdm_imei, sizeof(ictx.mdm_imei) - 1,
624+
*buf, 0, len);
625+
ictx.mdm_imei[out_len] = 0;
611626

612627
LOG_INF("IMEI: %s", ictx.mdm_imei);
613628
}
@@ -685,10 +700,12 @@ static void on_cmd_sockerror(struct net_buf **buf, u16_t len)
685700
static void on_cmd_sockexterror(struct net_buf **buf, u16_t len)
686701
{
687702
char value[8];
703+
size_t out_len;
688704

689705
struct wncm14a2a_socket *sock = NULL;
690706

691-
net_buf_linearize(value, sizeof(value), *buf, 0, len);
707+
out_len = net_buf_linearize(value, sizeof(value) - 1, *buf, 0, len);
708+
value[out_len] = 0;
692709
ictx.last_error = -atoi(value);
693710
LOG_ERR("@EXTERR:%d", ictx.last_error);
694711
sock = socket_from_id(ictx.last_socket_id);
@@ -703,8 +720,10 @@ static void on_cmd_sockexterror(struct net_buf **buf, u16_t len)
703720
static void on_cmd_sockdial(struct net_buf **buf, u16_t len)
704721
{
705722
char value[8];
723+
size_t out_len;
706724

707-
net_buf_linearize(value, sizeof(value), *buf, 0, len);
725+
out_len = net_buf_linearize(value, sizeof(value) - 1, *buf, 0, len);
726+
value[out_len] = 0;
708727
ictx.last_error = atoi(value);
709728
k_sem_give(&ictx.response_sem);
710729
}
@@ -729,11 +748,13 @@ static void on_cmd_sockcreat(struct net_buf **buf, u16_t len)
729748
static void on_cmd_sockwrite(struct net_buf **buf, u16_t len)
730749
{
731750
char value[8];
751+
size_t out_len;
732752
int write_len;
733753
struct wncm14a2a_socket *sock = NULL;
734754

735755
/* TODO: check against what we wanted to send */
736-
net_buf_linearize(value, sizeof(value), *buf, 0, len);
756+
out_len = net_buf_linearize(value, sizeof(value) - 1, *buf, 0, len);
757+
value[out_len] = 0;
737758
write_len = atoi(value);
738759
if (write_len <= 0) {
739760
return;
@@ -901,12 +922,14 @@ static void on_cmd_sockread(struct net_buf **buf, u16_t len)
901922
static void on_cmd_sockdataind(struct net_buf **buf, u16_t len)
902923
{
903924
int socket_id, session_status, left_bytes;
925+
size_t out_len;
904926
char *delim1, *delim2;
905927
char value[sizeof("#,#,#####\r")];
906928
char sendbuf[sizeof("AT@SOCKREAD=#,#####\r")];
907929
struct wncm14a2a_socket *sock = NULL;
908930

909-
net_buf_linearize(value, sizeof(value), *buf, 0, len);
931+
out_len = net_buf_linearize(value, sizeof(value) - 1, *buf, 0, len);
932+
value[out_len] = 0;
910933

911934
/* First comma separator marks the end of socket_id */
912935
delim1 = strchr(value, ',');
@@ -961,9 +984,11 @@ static void on_cmd_sockdataind(struct net_buf **buf, u16_t len)
961984
static void on_cmd_socknotifyev(struct net_buf **buf, u16_t len)
962985
{
963986
char value[40];
987+
size_t out_len;
964988
int p1 = 0, p2 = 0;
965989

966-
net_buf_linearize(value, sizeof(value), *buf, 0, len);
990+
out_len = net_buf_linearize(value, sizeof(value) - 1, *buf, 0, len);
991+
value[out_len] = 0;
967992

968993
/* walk value till 1st quote */
969994
while (p1 < len && value[p1] != '\"') {

subsys/net/buf.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -685,9 +685,6 @@ size_t net_buf_linearize(void *dst, size_t dst_len, struct net_buf *src,
685685

686686
frag = src;
687687

688-
/* clear dst */
689-
(void)memset(dst, 0, dst_len);
690-
691688
/* find the right fragment to start copying from */
692689
while (frag && offset >= frag->len) {
693690
offset -= frag->len;

subsys/net/lib/sockets/sockets.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,11 @@ static inline ssize_t zsock_recv_dgram(struct net_context *ctx,
531531
recv_len = max_len;
532532
}
533533

534-
net_frag_linearize(buf, recv_len, pkt, header_len, recv_len);
534+
/* Length passed as arguments are all based on packet data size
535+
* and output buffer size, so return value is invariantly == recv_len,
536+
* and we just ignore it.
537+
*/
538+
(void)net_frag_linearize(buf, recv_len, pkt, header_len, recv_len);
535539

536540
if (!(flags & ZSOCK_MSG_PEEK)) {
537541
net_pkt_unref(pkt);

0 commit comments

Comments
 (0)