[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 01/32] netlink: Avoid memcpy() across flexible array boundary
 
- To: "Gustavo A . R . Silva" <gustavoars@xxxxxxxxxx>
 
- From: Kees Cook <keescook@xxxxxxxxxxxx>
 
- Date: Tue,  3 May 2022 18:44:10 -0700
 
- Cc: Kees Cook <keescook@xxxxxxxxxxxx>,	Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>,	"David S. Miller" <davem@xxxxxxxxxxxxx>,	Jakub Kicinski <kuba@xxxxxxxxxx>,	Rich Felker <dalias@xxxxxxxxxx>,	Eric Dumazet <edumazet@xxxxxxxxxx>,	netdev@xxxxxxxxxxxxxxx,	Alexei Starovoitov <ast@xxxxxxxxxx>,	alsa-devel@xxxxxxxxxxxxxxxx,	Al Viro <viro@xxxxxxxxxxxxxxxxxx>,	Andrew Gabbasov <andrew_gabbasov@xxxxxxxxxx>,	Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>,	Andy Gross <agross@xxxxxxxxxx>,	Andy Lavr <andy.lavr@xxxxxxxxx>,	Arend van Spriel <aspriel@xxxxxxxxx>,	Baowen Zheng <baowen.zheng@xxxxxxxxxxxx>,	Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>,	Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>,	Bradley Grove <linuxdrivers@xxxxxxxxxxxx>,	brcm80211-dev-list.pdl@xxxxxxxxxxxx,	Christian Brauner <brauner@xxxxxxxxxx>,	Christian Göttsche <cgzones@xxxxxxxxxxxxxx>,	Christian Lamparter <chunkeey@xxxxxxxxxxxxxx>,	Chris Zankel <chris@xxxxxxxxxx>,	Cong Wang <cong.wang@xxxxxxxxxxxxx>,	Daniel Axtens <dja@xxxxxxxxxx>,	Daniel Vetter <daniel.vetter@xxxxxxxx>,	Dan Williams <dan.j.williams@xxxxxxxxx>,	David Gow <davidgow@xxxxxxxxxx>,	David Howells <dhowells@xxxxxxxxxx>,	Dennis Dalessandro <dennis.dalessandro@xxxxxxxxxxxxxxxxxxxx>,	devicetree@xxxxxxxxxxxxxxx,	Dexuan Cui <decui@xxxxxxxxxxxxx>,	Dmitry Kasatkin <dmitry.kasatkin@xxxxxxxxx>,	Eli Cohen <elic@xxxxxxxxxx>,	Eric Paris <eparis@xxxxxxxxxxxxxx>,	Eugeniu Rosca <erosca@xxxxxxxxxxxxxx>,	Felipe Balbi <balbi@xxxxxxxxxx>,	Francis Laniel <laniel_francis@xxxxxxxxxxxxxxxxxxx>,	Frank Rowand <frowand.list@xxxxxxxxx>,	Franky Lin <franky.lin@xxxxxxxxxxxx>,	Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>,	Gregory Greenman <gregory.greenman@xxxxxxxxx>,	Guenter Roeck <linux@xxxxxxxxxxxx>,	Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>,	Hante Meuleman <hante.meuleman@xxxxxxxxxxxx>,	Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>,	Hulk Robot <hulkci@xxxxxxxxxx>,	"James E.J. Bottomley" <jejb@xxxxxxxxxxxxx>,	James Morris <jmorris@xxxxxxxxx>,	Jarkko Sakkinen <jarkko@xxxxxxxxxx>,	Jaroslav Kysela <perex@xxxxxxxx>,	Jason Gunthorpe <jgg@xxxxxxxx>,	Jens Axboe <axboe@xxxxxxxxx>,	Johan Hedberg <johan.hedberg@xxxxxxxxx>,	Johannes Berg <johannes.berg@xxxxxxxxx>,	Johannes Berg <johannes@xxxxxxxxxxxxxxxx>,	John Keeping <john@xxxxxxxxxxxx>,	Juergen Gross <jgross@xxxxxxxx>,	Kalle Valo <kvalo@xxxxxxxxxx>,	Keith Packard <keithp@xxxxxxxxxx>,	keyrings@xxxxxxxxxxxxxxx,	kunit-dev@xxxxxxxxxxxxxxxx,	Kuniyuki Iwashima <kuniyu@xxxxxxxxxxxx>,	"K. Y. Srinivasan" <kys@xxxxxxxxxxxxx>,	Lars-Peter Clausen <lars@xxxxxxxxxx>,	Lee Jones <lee.jones@xxxxxxxxxx>,	Leon Romanovsky <leon@xxxxxxxxxx>,	Liam Girdwood <lgirdwood@xxxxxxxxx>,	linux1394-devel@xxxxxxxxxxxxxxxxxxxxx,	linux-afs@xxxxxxxxxxxxxxxxxxx,	linux-arm-kernel@xxxxxxxxxxxxxxxxxxx,	linux-arm-msm@xxxxxxxxxxxxxxx,	linux-bluetooth@xxxxxxxxxxxxxxx,	linux-hardening@xxxxxxxxxxxxxxx,	linux-hyperv@xxxxxxxxxxxxxxx,	linux-integrity@xxxxxxxxxxxxxxx,	linux-rdma@xxxxxxxxxxxxxxx,	linux-scsi@xxxxxxxxxxxxxxx,	linux-security-module@xxxxxxxxxxxxxxx,	linux-usb@xxxxxxxxxxxxxxx,	linux-wireless@xxxxxxxxxxxxxxx,	linux-xtensa@xxxxxxxxxxxxxxxx,	llvm@xxxxxxxxxxxxxxx,	Loic Poulain <loic.poulain@xxxxxxxxxx>,	Louis Peens <louis.peens@xxxxxxxxxxxx>,	Luca Coelho <luciano.coelho@xxxxxxxxx>,	Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>,	Marc Dionne <marc.dionne@xxxxxxxxxxxx>,	Marcel Holtmann <marcel@xxxxxxxxxxxx>,	Mark Brown <broonie@xxxxxxxxxx>,	"Martin K. Petersen" <martin.petersen@xxxxxxxxxx>,	Max Filippov <jcmvbkbc@xxxxxxxxx>,	Mimi Zohar <zohar@xxxxxxxxxxxxx>,	Muchun Song <songmuchun@xxxxxxxxxxxxx>,	Nathan Chancellor <nathan@xxxxxxxxxx>,	Nick Desaulniers <ndesaulniers@xxxxxxxxxx>,	Nuno Sá <nuno.sa@xxxxxxxxxx>,	Paolo Abeni <pabeni@xxxxxxxxxx>,	Paul Moore <paul@xxxxxxxxxxxxxx>,	Rob Herring <robh+dt@xxxxxxxxxx>,	Russell King <linux@xxxxxxxxxxxxxxx>,	selinux@xxxxxxxxxxxxxxx,	"Serge E. Hallyn" <serge@xxxxxxxxxx>,	SHA-cyfmac-dev-list@xxxxxxxxxxxx,	Simon Horman <simon.horman@xxxxxxxxxxxx>,	Stefano Stabellini <sstabellini@xxxxxxxxxx>,	Stefan Richter <stefanr@xxxxxxxxxxxxxxxxx>,	Steffen Klassert <steffen.klassert@xxxxxxxxxxx>,	Stephen Hemminger <sthemmin@xxxxxxxxxxxxx>,	Stephen Smalley <stephen.smalley.work@xxxxxxxxx>,	Tadeusz Struk <tadeusz.struk@xxxxxxxxxx>,	Takashi Iwai <tiwai@xxxxxxxx>,	Tom Rix <trix@xxxxxxxxxx>,	Udipto Goswami <quic_ugoswami@xxxxxxxxxxx>,	Vincenzo Frascino <vincenzo.frascino@xxxxxxx>,	wcn36xx@xxxxxxxxxxxxxxxxxxx,	Wei Liu <wei.liu@xxxxxxxxxx>,	xen-devel@xxxxxxxxxxxxxxxxxxxx,	Xiu Jianfeng <xiujianfeng@xxxxxxxxxx>,	Yang Yingliang <yangyingliang@xxxxxxxxxx>
 
- Delivery-date: Wed, 04 May 2022 05:16:19 +0000
 
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
 
 
 
In preparation for run-time memcpy() bounds checking, split the nlmsg
copying for error messages (which crosses a previous unspecified flexible
array boundary) in half. Avoids the future run-time warning:
memcpy: detected field-spanning write (size 32) of single field "&errmsg->msg" 
(size 16)
Creates an explicit flexible array at the end of nlmsghdr for the payload,
named "nlmsg_payload". There is no impact on UAPI; the sizeof(struct
nlmsghdr) does not change, but now the compiler can better reason about
where things are being copied.
Fixed-by: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
Link: 
https://lore.kernel.org/lkml/d7251d92-150b-5346-6237-52afc154bb00@xxxxxxxxxxxxxxxxxx
Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
Cc: Jakub Kicinski <kuba@xxxxxxxxxx>
Cc: Rich Felker <dalias@xxxxxxxxxx>
Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
Cc: netdev@xxxxxxxxxxxxxxx
Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
---
 include/uapi/linux/netlink.h | 1 +
 net/netlink/af_netlink.c     | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index 855dffb4c1c3..47f9342d51bc 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -47,6 +47,7 @@ struct nlmsghdr {
        __u16           nlmsg_flags;    /* Additional flags */
        __u32           nlmsg_seq;      /* Sequence number */
        __u32           nlmsg_pid;      /* Sending process port ID */
+       __u8            nlmsg_payload[];/* Contents of message */
 };
 
 /* Flags values */
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 1b5a9c2e1c29..09346aee1022 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2445,7 +2445,10 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr 
*nlh, int err,
                          NLMSG_ERROR, payload, flags);
        errmsg = nlmsg_data(rep);
        errmsg->error = err;
-       memcpy(&errmsg->msg, nlh, payload > sizeof(*errmsg) ? nlh->nlmsg_len : 
sizeof(*nlh));
+       errmsg->msg = *nlh;
+       if (payload > sizeof(*errmsg))
+               memcpy(errmsg->msg.nlmsg_payload, nlh->nlmsg_payload,
+                      nlh->nlmsg_len - sizeof(*nlh));
 
        if (nlk_has_extack && extack) {
                if (extack->_msg) {
-- 
2.32.0
 
    
     |