Skip to content

Commit 15d2540

Browse files
committed
tools: ynl: check for overflow of constructed messages
Donald points out that we don't check for overflows. Stash the length of the message on nlmsg_pid (nlmsg_seq would do as well). This allows the attribute helpers to remain self-contained (no extra arguments). Also let the put helpers continue to return nothing. The error is checked only in (newly introduced) ynl_msg_end(). Reviewed-by: Donald Hunter <donald.hunter@gmail.com> Link: https://lore.kernel.org/r/20240305185000.964773-1-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent e3afe5d commit 15d2540

File tree

3 files changed

+68
-4
lines changed

3 files changed

+68
-4
lines changed

tools/net/ynl/lib/ynl-priv.h

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,8 @@ int ynl_error_parse(struct ynl_parse_arg *yarg, const char *msg);
135135

136136
/* Netlink message handling helpers */
137137

138+
#define YNL_MSG_OVERFLOW 1
139+
138140
static inline struct nlmsghdr *ynl_nlmsg_put_header(void *buf)
139141
{
140142
struct nlmsghdr *nlh = buf;
@@ -239,11 +241,29 @@ ynl_attr_first(const void *start, size_t len, size_t skip)
239241
return ynl_attr_if_good(start + len, attr);
240242
}
241243

244+
static inline bool
245+
__ynl_attr_put_overflow(struct nlmsghdr *nlh, size_t size)
246+
{
247+
bool o;
248+
249+
/* ynl_msg_start() stashed buffer length in nlmsg_pid. */
250+
o = nlh->nlmsg_len + NLA_HDRLEN + NLMSG_ALIGN(size) > nlh->nlmsg_pid;
251+
if (o)
252+
/* YNL_MSG_OVERFLOW is < NLMSG_HDRLEN, all subsequent checks
253+
* are guaranteed to fail.
254+
*/
255+
nlh->nlmsg_pid = YNL_MSG_OVERFLOW;
256+
return o;
257+
}
258+
242259
static inline struct nlattr *
243260
ynl_attr_nest_start(struct nlmsghdr *nlh, unsigned int attr_type)
244261
{
245262
struct nlattr *attr;
246263

264+
if (__ynl_attr_put_overflow(nlh, 0))
265+
return ynl_nlmsg_end_addr(nlh) - NLA_HDRLEN;
266+
247267
attr = ynl_nlmsg_end_addr(nlh);
248268
attr->nla_type = attr_type | NLA_F_NESTED;
249269
nlh->nlmsg_len += NLA_HDRLEN;
@@ -263,6 +283,9 @@ ynl_attr_put(struct nlmsghdr *nlh, unsigned int attr_type,
263283
{
264284
struct nlattr *attr;
265285

286+
if (__ynl_attr_put_overflow(nlh, size))
287+
return;
288+
266289
attr = ynl_nlmsg_end_addr(nlh);
267290
attr->nla_type = attr_type;
268291
attr->nla_len = NLA_HDRLEN + size;
@@ -276,14 +299,17 @@ static inline void
276299
ynl_attr_put_str(struct nlmsghdr *nlh, unsigned int attr_type, const char *str)
277300
{
278301
struct nlattr *attr;
279-
const char *end;
302+
size_t len;
303+
304+
len = strlen(str);
305+
if (__ynl_attr_put_overflow(nlh, len))
306+
return;
280307

281308
attr = ynl_nlmsg_end_addr(nlh);
282309
attr->nla_type = attr_type;
283310

284-
end = stpcpy(ynl_attr_data(attr), str);
285-
attr->nla_len =
286-
NLA_HDRLEN + NLA_ALIGN(end - (char *)ynl_attr_data(attr));
311+
strcpy(ynl_attr_data(attr), str);
312+
attr->nla_len = NLA_HDRLEN + NLA_ALIGN(len);
287313

288314
nlh->nlmsg_len += NLMSG_ALIGN(attr->nla_len);
289315
}

tools/net/ynl/lib/ynl.c

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,9 +404,33 @@ struct nlmsghdr *ynl_msg_start(struct ynl_sock *ys, __u32 id, __u16 flags)
404404
nlh->nlmsg_flags = flags;
405405
nlh->nlmsg_seq = ++ys->seq;
406406

407+
/* This is a local YNL hack for length checking, we put the buffer
408+
* length in nlmsg_pid, since messages sent to the kernel always use
409+
* PID 0. Message needs to be terminated with ynl_msg_end().
410+
*/
411+
nlh->nlmsg_pid = YNL_SOCKET_BUFFER_SIZE;
412+
407413
return nlh;
408414
}
409415

416+
static int ynl_msg_end(struct ynl_sock *ys, struct nlmsghdr *nlh)
417+
{
418+
/* We stash buffer length in nlmsg_pid. */
419+
if (nlh->nlmsg_pid == 0) {
420+
yerr(ys, YNL_ERROR_INPUT_INVALID,
421+
"Unknown input buffer length");
422+
return -EINVAL;
423+
}
424+
if (nlh->nlmsg_pid == YNL_MSG_OVERFLOW) {
425+
yerr(ys, YNL_ERROR_INPUT_TOO_BIG,
426+
"Constructred message longer than internal buffer");
427+
return -EMSGSIZE;
428+
}
429+
430+
nlh->nlmsg_pid = 0;
431+
return 0;
432+
}
433+
410434
struct nlmsghdr *
411435
ynl_gemsg_start(struct ynl_sock *ys, __u32 id, __u16 flags,
412436
__u8 cmd, __u8 version)
@@ -607,6 +631,10 @@ static int ynl_sock_read_family(struct ynl_sock *ys, const char *family_name)
607631
nlh = ynl_gemsg_start_req(ys, GENL_ID_CTRL, CTRL_CMD_GETFAMILY, 1);
608632
ynl_attr_put_str(nlh, CTRL_ATTR_FAMILY_NAME, family_name);
609633

634+
err = ynl_msg_end(ys, nlh);
635+
if (err < 0)
636+
return err;
637+
610638
err = send(ys->socket, nlh, nlh->nlmsg_len, 0);
611639
if (err < 0) {
612640
perr(ys, "failed to request socket family info");
@@ -868,6 +896,10 @@ int ynl_exec(struct ynl_sock *ys, struct nlmsghdr *req_nlh,
868896
{
869897
int err;
870898

899+
err = ynl_msg_end(ys, req_nlh);
900+
if (err < 0)
901+
return err;
902+
871903
err = send(ys->socket, req_nlh, req_nlh->nlmsg_len, 0);
872904
if (err < 0)
873905
return err;
@@ -921,6 +953,10 @@ int ynl_exec_dump(struct ynl_sock *ys, struct nlmsghdr *req_nlh,
921953
{
922954
int err;
923955

956+
err = ynl_msg_end(ys, req_nlh);
957+
if (err < 0)
958+
return err;
959+
924960
err = send(ys->socket, req_nlh, req_nlh->nlmsg_len, 0);
925961
if (err < 0)
926962
return err;

tools/net/ynl/lib/ynl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ enum ynl_error_code {
2020
YNL_ERROR_ATTR_INVALID,
2121
YNL_ERROR_UNKNOWN_NTF,
2222
YNL_ERROR_INV_RESP,
23+
YNL_ERROR_INPUT_INVALID,
24+
YNL_ERROR_INPUT_TOO_BIG,
2325
};
2426

2527
/**

0 commit comments

Comments
 (0)