[PATCH] Bluetooth: Add sockaddr length checks before accessing sa_family in bind and connect handlers

2017-06-29 Thread Mateusz Jurczyk
Verify that the caller-provided sockaddr structure is large enough to
contain the sa_family field, before accessing it in bind() and connect()
handlers of the Bluetooth sockets. Since neither syscall enforces a minimum
size of the corresponding memory region, very short sockaddrs (zero or one
byte long) result in operating on uninitialized memory while referencing
sa_family.

Signed-off-by: Mateusz Jurczyk 
---
 net/bluetooth/l2cap_sock.c  | 5 +++--
 net/bluetooth/rfcomm/sock.c | 3 ++-
 net/bluetooth/sco.c | 6 ++
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 507b80d59dec..67a8642f57ea 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -87,7 +87,8 @@ static int l2cap_sock_bind(struct socket *sock, struct 
sockaddr *addr, int alen)
 
BT_DBG("sk %p", sk);
 
-   if (!addr || addr->sa_family != AF_BLUETOOTH)
+   if (!addr || alen < offsetofend(struct sockaddr, sa_family) ||
+   addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
 
memset(&la, 0, sizeof(la));
@@ -181,7 +182,7 @@ static int l2cap_sock_connect(struct socket *sock, struct 
sockaddr *addr,
 
BT_DBG("sk %p", sk);
 
-   if (!addr || alen < sizeof(addr->sa_family) ||
+   if (!addr || alen < offsetofend(struct sockaddr, sa_family) ||
addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
 
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index ac3c650cb234..2025b45a8bf8 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -339,7 +339,8 @@ static int rfcomm_sock_bind(struct socket *sock, struct 
sockaddr *addr, int addr
struct sock *sk = sock->sk;
int len, err = 0;
 
-   if (!addr || addr->sa_family != AF_BLUETOOTH)
+   if (!addr || addr_len < offsetofend(struct sockaddr, sa_family) ||
+   addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
 
memset(&sa, 0, sizeof(sa));
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 728e0c8dc8e7..795e920a3281 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -524,10 +524,8 @@ static int sco_sock_bind(struct socket *sock, struct 
sockaddr *addr,
 
BT_DBG("sk %p %pMR", sk, &sa->sco_bdaddr);
 
-   if (!addr || addr->sa_family != AF_BLUETOOTH)
-   return -EINVAL;
-
-   if (addr_len < sizeof(struct sockaddr_sco))
+   if (!addr || addr_len < sizeof(struct sockaddr_sco) ||
+   addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
 
lock_sock(sk);
-- 
2.13.2.725.g09c95d1e9-goog



Re: [PATCH v2] netfilter: nfnetlink: Improve input length sanitization in nfnetlink_rcv

2017-06-30 Thread Mateusz Jurczyk
On Thu, Jun 29, 2017 at 6:22 PM, Pablo Neira Ayuso  wrote:
> On Tue, Jun 27, 2017 at 07:05:27PM +0200, Pablo Neira Ayuso wrote:
>> On Tue, Jun 27, 2017 at 05:58:25PM +0200, Pablo Neira Ayuso wrote:
>> > On Wed, Jun 07, 2017 at 03:50:38PM +0200, Mateusz Jurczyk wrote:
>> > > Verify that the length of the socket buffer is sufficient to cover the
>> > > nlmsghdr structure before accessing the nlh->nlmsg_len field for further
>> > > input sanitization. If the client only supplies 1-3 bytes of data in
>> > > sk_buff, then nlh->nlmsg_len remains partially uninitialized and
>> > > contains leftover memory from the corresponding kernel allocation.
>> > > Operating on such data may result in indeterminate evaluation of the
>> > > nlmsg_len < NLMSG_HDRLEN expression.
>> > >
>> > > The bug was discovered by a runtime instrumentation designed to detect
>> > > use of uninitialized memory in the kernel. The patch prevents this and
>> > > other similar tools (e.g. KMSAN) from flagging this behavior in the 
>> > > future.
>> >
>> > Applied, thanks.
>>
>> Wait, I keeping this back after closer look.
>>
>> I think we have to remove this:
>>
>> if (nlh->nlmsg_len < NLMSG_HDRLEN || <---
>> skb->len < NLMSG_HDRLEN + sizeof(struct nfgenmsg))
>> return;
>>
>> in nfnetlink_rcv_skb_batch()
>>
>> now that we make this unfront check from nfnetlink_rcv().
>
> BTW, I can just mangle your patch here to delete such line to speed up
> things. See the mangled patch that is attached to this email.

Sure, I think the condition in nfnetlink_rcv_skb_batch() can be now
safely removed. Feel free to proceed with the mangled patch. Thanks.


[PATCH] netfilter: nfnetlink: Improve input length sanitization in nfnetlink_rcv

2017-06-07 Thread Mateusz Jurczyk
Verify that the length of the socket buffer is sufficient to cover the
entire nlh->nlmsg_len field before accessing that field for further
input sanitization. If the client only supplies 1-3 bytes of data in
sk_buff, then nlh->nlmsg_len remains partially uninitialized and
contains leftover memory from the corresponding kernel allocation.
Operating on such data may result in indeterminate evaluation of the
nlmsg_len < NLMSG_HDRLEN expression.

The bug was discovered by a runtime instrumentation designed to detect
use of uninitialized memory in the kernel. The patch prevents this and
other similar tools (e.g. KMSAN) from flagging this behavior in the future.

Signed-off-by: Mateusz Jurczyk 
---
 net/netfilter/nfnetlink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 80f5ecf2c3d7..c634cfca40ec 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -491,7 +491,8 @@ static void nfnetlink_rcv(struct sk_buff *skb)
 {
struct nlmsghdr *nlh = nlmsg_hdr(skb);
 
-   if (nlh->nlmsg_len < NLMSG_HDRLEN ||
+   if (skb->len < sizeof(nlh->nlmsg_len) ||
+   nlh->nlmsg_len < NLMSG_HDRLEN ||
skb->len < nlh->nlmsg_len)
return;
 
-- 
2.13.1.508.gb3defc5cc-goog



[PATCH] decnet: dn_rtmsg: Improve input length sanitization in dnrmg_receive_user_skb

2017-06-07 Thread Mateusz Jurczyk
Verify that the length of the socket buffer is sufficient to cover the
entire nlh->nlmsg_len field before accessing that field for further
input sanitization. If the client only supplies 1-3 bytes of data in
sk_buff, then nlh->nlmsg_len remains partially uninitialized and
contains leftover memory from the corresponding kernel allocation.
Operating on such data may result in indeterminate evaluation of the
nlmsg_len < sizeof(*nlh) expression.

The bug was discovered by a runtime instrumentation designed to detect
use of uninitialized memory in the kernel. The patch prevents this and
other similar tools (e.g. KMSAN) from flagging this behavior in the future.

Signed-off-by: Mateusz Jurczyk 
---
 net/decnet/netfilter/dn_rtmsg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/decnet/netfilter/dn_rtmsg.c b/net/decnet/netfilter/dn_rtmsg.c
index 1ed81ac6dd1a..26e020e9d415 100644
--- a/net/decnet/netfilter/dn_rtmsg.c
+++ b/net/decnet/netfilter/dn_rtmsg.c
@@ -102,7 +102,9 @@ static inline void dnrmg_receive_user_skb(struct sk_buff 
*skb)
 {
struct nlmsghdr *nlh = nlmsg_hdr(skb);
 
-   if (nlh->nlmsg_len < sizeof(*nlh) || skb->len < nlh->nlmsg_len)
+   if (skb->len < sizeof(nlh->nlmsg_len) ||
+   nlh->nlmsg_len < sizeof(*nlh) ||
+   skb->len < nlh->nlmsg_len)
return;
 
if (!netlink_capable(skb, CAP_NET_ADMIN))
-- 
2.13.1.508.gb3defc5cc-goog



[PATCH v2] netfilter: nfnetlink: Improve input length sanitization in nfnetlink_rcv

2017-06-07 Thread Mateusz Jurczyk
Verify that the length of the socket buffer is sufficient to cover the
nlmsghdr structure before accessing the nlh->nlmsg_len field for further
input sanitization. If the client only supplies 1-3 bytes of data in
sk_buff, then nlh->nlmsg_len remains partially uninitialized and
contains leftover memory from the corresponding kernel allocation.
Operating on such data may result in indeterminate evaluation of the
nlmsg_len < NLMSG_HDRLEN expression.

The bug was discovered by a runtime instrumentation designed to detect
use of uninitialized memory in the kernel. The patch prevents this and
other similar tools (e.g. KMSAN) from flagging this behavior in the future.

Signed-off-by: Mateusz Jurczyk 
---
Changes in v2:
  - Compare skb->len against NLMSG_HDRLEN to avoid assuming the layout of
the nlmsghdr structure.

 net/netfilter/nfnetlink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 80f5ecf2c3d7..1f9667f52be5 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -491,7 +491,8 @@ static void nfnetlink_rcv(struct sk_buff *skb)
 {
struct nlmsghdr *nlh = nlmsg_hdr(skb);
 
-   if (nlh->nlmsg_len < NLMSG_HDRLEN ||
+   if (skb->len < NLMSG_HDRLEN ||
+   nlh->nlmsg_len < NLMSG_HDRLEN ||
skb->len < nlh->nlmsg_len)
return;
 
-- 
2.13.1.508.gb3defc5cc-goog



[PATCH v2] decnet: dn_rtmsg: Improve input length sanitization in dnrmg_receive_user_skb

2017-06-07 Thread Mateusz Jurczyk
Verify that the length of the socket buffer is sufficient to cover the
nlmsghdr structure before accessing the nlh->nlmsg_len field for further
input sanitization. If the client only supplies 1-3 bytes of data in
sk_buff, then nlh->nlmsg_len remains partially uninitialized and
contains leftover memory from the corresponding kernel allocation.
Operating on such data may result in indeterminate evaluation of the
nlmsg_len < sizeof(*nlh) expression.

The bug was discovered by a runtime instrumentation designed to detect
use of uninitialized memory in the kernel. The patch prevents this and
other similar tools (e.g. KMSAN) from flagging this behavior in the future.

Signed-off-by: Mateusz Jurczyk 
---
Changes in v2:
  - Compare skb->len against sizeof(*nlh) instead of sizeof(nlh->nlmsg_len)
to avoid assuming the layout of the nlmsghdr structure. This was
motivated by Eric Dumazet's comment on a related patch submission.

 net/decnet/netfilter/dn_rtmsg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/decnet/netfilter/dn_rtmsg.c b/net/decnet/netfilter/dn_rtmsg.c
index 1ed81ac6dd1a..aa8ffecc46a4 100644
--- a/net/decnet/netfilter/dn_rtmsg.c
+++ b/net/decnet/netfilter/dn_rtmsg.c
@@ -102,7 +102,9 @@ static inline void dnrmg_receive_user_skb(struct sk_buff 
*skb)
 {
struct nlmsghdr *nlh = nlmsg_hdr(skb);
 
-   if (nlh->nlmsg_len < sizeof(*nlh) || skb->len < nlh->nlmsg_len)
+   if (skb->len < sizeof(*nlh) ||
+   nlh->nlmsg_len < sizeof(*nlh) ||
+   skb->len < nlh->nlmsg_len)
return;
 
if (!netlink_capable(skb, CAP_NET_ADMIN))
-- 
2.13.1.508.gb3defc5cc-goog



Re: [PATCH v2] decnet: dn_rtmsg: Improve input length sanitization in dnrmg_receive_user_skb

2017-06-07 Thread Mateusz Jurczyk
On Wed, Jun 7, 2017 at 4:18 PM, Florian Westphal  wrote:
> Mateusz Jurczyk  wrote:
>> Verify that the length of the socket buffer is sufficient to cover the
>> nlmsghdr structure before accessing the nlh->nlmsg_len field for further
>> input sanitization. If the client only supplies 1-3 bytes of data in
>> sk_buff, then nlh->nlmsg_len remains partially uninitialized and
>> contains leftover memory from the corresponding kernel allocation.
>> Operating on such data may result in indeterminate evaluation of the
>> nlmsg_len < sizeof(*nlh) expression.
>>
>> The bug was discovered by a runtime instrumentation designed to detect
>> use of uninitialized memory in the kernel. The patch prevents this and
>> other similar tools (e.g. KMSAN) from flagging this behavior in the future.
>
> Instead of changing all the internal users wouldn't it be better
> to add this check once in netlink_unicast_kernel?
>

Perhaps. I must admit I'm not very familiar with this code
area/interface, so I preferred to fix the few specific cases instead
of submitting a general patch, which might have some unexpected side
effects, e.g. behavior different from one of the internal clients etc.

If you think one check in netlink_unicast_kernel is a better way to do
it, I'm happy to implement it like that.

Thanks,
Mateusz


[PATCH] af_unix: Add sockaddr length checks before accessing sa_family in bind and connect handlers

2017-06-08 Thread Mateusz Jurczyk
Verify that the caller-provided sockaddr structure is large enough to
contain the sa_family field, before accessing it in bind() and connect()
handlers of the AF_UNIX socket. Since neither syscall enforces a minimum
size of the corresponding memory region, very short sockaddrs (zero or
one byte long) result in operating on uninitialized memory while
referencing .sa_family.

Signed-off-by: Mateusz Jurczyk 
---
 net/unix/af_unix.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 6a7fe7660551..1a0c961f4ffe 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -999,7 +999,8 @@ static int unix_bind(struct socket *sock, struct sockaddr 
*uaddr, int addr_len)
struct path path = { };
 
err = -EINVAL;
-   if (sunaddr->sun_family != AF_UNIX)
+   if (addr_len < offsetofend(struct sockaddr_un, sun_family) ||
+   sunaddr->sun_family != AF_UNIX)
goto out;
 
if (addr_len == sizeof(short)) {
@@ -1110,6 +,10 @@ static int unix_dgram_connect(struct socket *sock, 
struct sockaddr *addr,
unsigned int hash;
int err;
 
+   err = -EINVAL;
+   if (alen < offsetofend(struct sockaddr, sa_family))
+   goto out;
+
if (addr->sa_family != AF_UNSPEC) {
err = unix_mkname(sunaddr, alen, &hash);
if (err < 0)
-- 
2.13.1.508.gb3defc5cc-goog



Re: [PATCH] af_unix: Add sockaddr length checks before accessing sa_family in bind and connect handlers

2017-06-09 Thread Mateusz Jurczyk
On Thu, Jun 8, 2017 at 10:04 PM, David Miller  wrote:
> From: Mateusz Jurczyk 
> Date: Thu,  8 Jun 2017 11:13:36 +0200
>
>> Verify that the caller-provided sockaddr structure is large enough to
>> contain the sa_family field, before accessing it in bind() and connect()
>> handlers of the AF_UNIX socket. Since neither syscall enforces a minimum
>> size of the corresponding memory region, very short sockaddrs (zero or
>> one byte long) result in operating on uninitialized memory while
>> referencing .sa_family.
>>
>> Signed-off-by: Mateusz Jurczyk 
>
> The sockaddr comes from a structure on the caller's kernel stack, even
> if the user gives a smaller length, it is legal to access that memory.

It is legal to access it, but since it's uninitialized kernel stack
memory, the results of comparisons against AF_UNIX or AF_UNSPEC are
indeterminate. In practice a user-mode program could likely use timing
measurement to infer the evaluation of these comparisons, and hence
determine if a garbage 16-bit variable on the kernel stack is equal to
0x or 0x0001, or a garbage byte is equal to 0x00 (if the first
byte is provided).

This is of course not very bad. However, my project for finding use of
uninitialized memory flagged it, and I thought it was worth fixing, at
least to avoid having this construct detected in the future (e.g. by
KMSAN).

There are a few more instances of this behavior in other socket types,
which I was going to report with separate patches. If you decide this
kind of issues indeed deserves a fix, please let me know if further
separate patches are the right approach.

Thanks,
Mateusz


[PATCH] nfc: Add sockaddr length checks before accessing sa_family in bind handlers

2017-06-13 Thread Mateusz Jurczyk
Verify that the caller-provided sockaddr structure is large enough to
contain the sa_family field, before accessing it in bind() handlers of the
AF_NFC socket. Since the syscall doesn't enforce a minimum size of the
corresponding memory region, very short sockaddrs (zero or one byte long)
result in operating on uninitialized memory while referencing .sa_family.

Signed-off-by: Mateusz Jurczyk 
---
 net/nfc/llcp_sock.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index 2ffb18e73df6..80cf35ed320a 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -77,7 +77,8 @@ static int llcp_sock_bind(struct socket *sock, struct 
sockaddr *addr, int alen)
struct sockaddr_nfc_llcp llcp_addr;
int len, ret = 0;
 
-   if (!addr || addr->sa_family != AF_NFC)
+   if (!addr || alen < offsetofend(struct sockaddr, sa_family) ||
+   addr->sa_family != AF_NFC)
return -EINVAL;
 
pr_debug("sk %p addr %p family %d\n", sk, addr, addr->sa_family);
@@ -151,7 +152,8 @@ static int llcp_raw_sock_bind(struct socket *sock, struct 
sockaddr *addr,
struct sockaddr_nfc_llcp llcp_addr;
int len, ret = 0;
 
-   if (!addr || addr->sa_family != AF_NFC)
+   if (!addr || alen < offsetofend(struct sockaddr, sa_family) ||
+   addr->sa_family != AF_NFC)
return -EINVAL;
 
pr_debug("sk %p addr %p family %d\n", sk, addr, addr->sa_family);
-- 
2.13.1.508.gb3defc5cc-goog



[PATCH] af_iucv: Move sockaddr length checks to before accessing sa_family in bind and connect handlers

2017-06-13 Thread Mateusz Jurczyk
Verify that the caller-provided sockaddr structure is large enough to
contain the sa_family field, before accessing it in bind() and connect()
handlers of the AF_IUCV socket. Since neither syscall enforces a minimum
size of the corresponding memory region, very short sockaddrs (zero or
one byte long) result in operating on uninitialized memory while
referencing .sa_family.

Signed-off-by: Mateusz Jurczyk 
---
 net/iucv/af_iucv.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index 84de7b6326dc..1f63e704ab50 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -716,10 +716,8 @@ static int iucv_sock_bind(struct socket *sock, struct 
sockaddr *addr,
char uid[9];
 
/* Verify the input sockaddr */
-   if (!addr || addr->sa_family != AF_IUCV)
-   return -EINVAL;
-
-   if (addr_len < sizeof(struct sockaddr_iucv))
+   if (!addr || addr_len < sizeof(struct sockaddr_iucv) ||
+   addr->sa_family != AF_IUCV)
return -EINVAL;
 
lock_sock(sk);
@@ -863,7 +861,7 @@ static int iucv_sock_connect(struct socket *sock, struct 
sockaddr *addr,
struct iucv_sock *iucv = iucv_sk(sk);
int err;
 
-   if (addr->sa_family != AF_IUCV || alen < sizeof(struct sockaddr_iucv))
+   if (alen < sizeof(struct sockaddr_iucv) || addr->sa_family != AF_IUCV)
return -EINVAL;
 
if (sk->sk_state != IUCV_OPEN && sk->sk_state != IUCV_BOUND)
-- 
2.13.1.508.gb3defc5cc-goog



[PATCH] caif: Add sockaddr length check before accessing sa_family in connect handler

2017-06-13 Thread Mateusz Jurczyk
Verify that the caller-provided sockaddr structure is large enough to
contain the sa_family field, before accessing it in the connect()
handler of the AF_CAIF socket. Since the syscall doesn't enforce a minimum
size of the corresponding memory region, very short sockaddrs (zero or one
byte long) result in operating on uninitialized memory while referencing
sa_family.

Signed-off-by: Mateusz Jurczyk 
---
 net/caif/caif_socket.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index adcad344c843..21f18ea2fce4 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -754,6 +754,10 @@ static int caif_connect(struct socket *sock, struct 
sockaddr *uaddr,
 
lock_sock(sk);
 
+   err = -EINVAL;
+   if (addr_len < offsetofend(struct sockaddr, sa_family))
+   goto out;
+
err = -EAFNOSUPPORT;
if (uaddr->sa_family != AF_CAIF)
goto out;
-- 
2.13.1.508.gb3defc5cc-goog



[PATCH] nfc: Fix the sockaddr length sanitization in llcp_sock_connect

2017-05-24 Thread Mateusz Jurczyk
Fix the sockaddr length verification in the connect() handler of NFC/LLCP
sockets, to compare against the size of the actual structure expected on
input (sockaddr_nfc_llcp) instead of its shorter version (sockaddr_nfc).

Both structures are defined in include/uapi/linux/nfc.h. The fields
specific to the _llcp extended struct are as follows:

   276  __u8 dsap; /* Destination SAP, if known */
   277  __u8 ssap; /* Source SAP to be bound to */
   278  char service_name[NFC_LLCP_MAX_SERVICE_NAME]; /* Service name 
URI */;
   279  size_t service_name_len;

If the caller doesn't provide a sufficiently long sockaddr buffer, these
fields remain uninitialized (and they currently originate from the stack
frame of the top-level sys_connect handler). They are then copied by
llcp_sock_connect() into internal storage (nfc_llcp_sock structure), and
could be subsequently read back through the user-mode getsockname()
function (handled by llcp_sock_getname()). This would result in the
disclosure of up to ~70 uninitialized bytes from the kernel stack to
user-mode clients capable of creating AFC_NFC sockets.

Signed-off-by: Mateusz Jurczyk 
---
 net/nfc/llcp_sock.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index 2ffb18e73df6..d0d12bea65cb 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -662,8 +662,7 @@ static int llcp_sock_connect(struct socket *sock, struct 
sockaddr *_addr,
 
pr_debug("sock %p sk %p flags 0x%x\n", sock, sk, flags);
 
-   if (!addr || len < sizeof(struct sockaddr_nfc) ||
-   addr->sa_family != AF_NFC)
+   if (!addr || len < sizeof(*addr) || addr->sa_family != AF_NFC)
return -EINVAL;
 
if (addr->service_name_len == 0 && addr->dsap == 0)
-- 
2.13.0.219.gdb65acc882-goog



[PATCH] nfc: Ensure presence of required attributes in the activate_target netlink handler

2017-05-24 Thread Mateusz Jurczyk
Check that the NFC_ATTR_TARGET_INDEX and NFC_ATTR_PROTOCOLS attributes (in
addition to NFC_ATTR_DEVICE_INDEX) are provided by the netlink client
prior to accessing them. This prevents potential unhandled NULL pointer
dereference exceptions which can be triggered by malicious user-mode
programs, if they omit one or both of these attributes.

Signed-off-by: Mateusz Jurczyk 
---
 net/nfc/netlink.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/nfc/netlink.c b/net/nfc/netlink.c
index 6b0850e63e09..b251fb936a27 100644
--- a/net/nfc/netlink.c
+++ b/net/nfc/netlink.c
@@ -907,7 +907,9 @@ static int nfc_genl_activate_target(struct sk_buff *skb, 
struct genl_info *info)
u32 device_idx, target_idx, protocol;
int rc;
 
-   if (!info->attrs[NFC_ATTR_DEVICE_INDEX])
+   if (!info->attrs[NFC_ATTR_DEVICE_INDEX] ||
+   !info->attrs[NFC_ATTR_TARGET_INDEX] ||
+   !info->attrs[NFC_ATTR_PROTOCOLS])
return -EINVAL;
 
device_idx = nla_get_u32(info->attrs[NFC_ATTR_DEVICE_INDEX]);
-- 
2.13.0.219.gdb65acc882-goog