lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20181007105323.hibmy6duhzu6h37w@brauner.io>
Date:   Sun, 7 Oct 2018 12:53:24 +0200
From:   Christian Brauner <christian@...uner.io>
To:     David Ahern <dsahern@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, jbenc@...hat.com,
        stephen@...workplumber.org, David Ahern <dsahern@...il.com>
Subject: Re: [PATCH net-next 19/20] net: Update netconf dump handlers for
 strict data checking

On Thu, Oct 04, 2018 at 02:33:54PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@...il.com>
> 
> Update inet_netconf_dump_devconf, inet6_netconf_dump_devconf, and
> mpls_netconf_dump_devconf for strict data checking. If the flag is set,
> the dump request is expected to have an netconfmsg struct as the header.
> The struct only has the family member and no attributes can be appended.
> 
> Signed-off-by: David Ahern <dsahern@...il.com>

Acked-by: Christian Brauner <christian@...uner.io>

> ---
>  net/ipv4/devinet.c  | 22 +++++++++++++++++++---
>  net/ipv6/addrconf.c | 22 +++++++++++++++++++---
>  net/mpls/af_mpls.c  | 18 +++++++++++++++++-
>  3 files changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index af968d4fe4fc..595706d6b672 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -2069,6 +2069,7 @@ static int inet_netconf_get_devconf(struct sk_buff *in_skb,
>  static int inet_netconf_dump_devconf(struct sk_buff *skb,
>  				     struct netlink_callback *cb)
>  {
> +	const struct nlmsghdr *nlh = cb->nlh;
>  	struct net *net = sock_net(skb->sk);
>  	int h, s_h;
>  	int idx, s_idx;
> @@ -2076,6 +2077,21 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
>  	struct in_device *in_dev;
>  	struct hlist_head *head;
>  
> +	if (cb->strict_check) {
> +		struct netlink_ext_ack *extack = cb->extack;
> +		struct netconfmsg *ncm;
> +
> +		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ncm))) {
> +			NL_SET_ERR_MSG(extack, "Invalid header");
> +			return -EINVAL;
> +		}
> +
> +		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ncm))) {
> +			NL_SET_ERR_MSG(extack, "Invalid data after header");
> +			return -EINVAL;
> +		}

Hm, I think this could just be one branch with !=
But if you've done this to report back a more meaningful error message
to userspace, fine. :)

> +	}
> +
>  	s_h = cb->args[0];
>  	s_idx = idx = cb->args[1];
>  
> @@ -2095,7 +2111,7 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
>  			if (inet_netconf_fill_devconf(skb, dev->ifindex,
>  						      &in_dev->cnf,
>  						      NETLINK_CB(cb->skb).portid,
> -						      cb->nlh->nlmsg_seq,
> +						      nlh->nlmsg_seq,
>  						      RTM_NEWNETCONF,
>  						      NLM_F_MULTI,
>  						      NETCONFA_ALL) < 0) {
> @@ -2112,7 +2128,7 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
>  		if (inet_netconf_fill_devconf(skb, NETCONFA_IFINDEX_ALL,
>  					      net->ipv4.devconf_all,
>  					      NETLINK_CB(cb->skb).portid,
> -					      cb->nlh->nlmsg_seq,
> +					      nlh->nlmsg_seq,
>  					      RTM_NEWNETCONF, NLM_F_MULTI,
>  					      NETCONFA_ALL) < 0)
>  			goto done;
> @@ -2123,7 +2139,7 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
>  		if (inet_netconf_fill_devconf(skb, NETCONFA_IFINDEX_DEFAULT,
>  					      net->ipv4.devconf_dflt,
>  					      NETLINK_CB(cb->skb).portid,
> -					      cb->nlh->nlmsg_seq,
> +					      nlh->nlmsg_seq,
>  					      RTM_NEWNETCONF, NLM_F_MULTI,
>  					      NETCONFA_ALL) < 0)
>  			goto done;
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 693199a29426..9dfe6c2106c1 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -666,6 +666,7 @@ static int inet6_netconf_get_devconf(struct sk_buff *in_skb,
>  static int inet6_netconf_dump_devconf(struct sk_buff *skb,
>  				      struct netlink_callback *cb)
>  {
> +	const struct nlmsghdr *nlh = cb->nlh;
>  	struct net *net = sock_net(skb->sk);
>  	int h, s_h;
>  	int idx, s_idx;
> @@ -673,6 +674,21 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
>  	struct inet6_dev *idev;
>  	struct hlist_head *head;
>  
> +	if (cb->strict_check) {
> +		struct netlink_ext_ack *extack = cb->extack;
> +		struct netconfmsg *ncm;
> +
> +		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ncm))) {
> +			NL_SET_ERR_MSG(extack, "Invalid header");
> +			return -EINVAL;
> +		}
> +
> +		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ncm))) {
> +			NL_SET_ERR_MSG(extack, "Invalid data after header");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	s_h = cb->args[0];
>  	s_idx = idx = cb->args[1];
>  
> @@ -692,7 +708,7 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
>  			if (inet6_netconf_fill_devconf(skb, dev->ifindex,
>  						       &idev->cnf,
>  						       NETLINK_CB(cb->skb).portid,
> -						       cb->nlh->nlmsg_seq,
> +						       nlh->nlmsg_seq,
>  						       RTM_NEWNETCONF,
>  						       NLM_F_MULTI,
>  						       NETCONFA_ALL) < 0) {
> @@ -709,7 +725,7 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
>  		if (inet6_netconf_fill_devconf(skb, NETCONFA_IFINDEX_ALL,
>  					       net->ipv6.devconf_all,
>  					       NETLINK_CB(cb->skb).portid,
> -					       cb->nlh->nlmsg_seq,
> +					       nlh->nlmsg_seq,
>  					       RTM_NEWNETCONF, NLM_F_MULTI,
>  					       NETCONFA_ALL) < 0)
>  			goto done;
> @@ -720,7 +736,7 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
>  		if (inet6_netconf_fill_devconf(skb, NETCONFA_IFINDEX_DEFAULT,
>  					       net->ipv6.devconf_dflt,
>  					       NETLINK_CB(cb->skb).portid,
> -					       cb->nlh->nlmsg_seq,
> +					       nlh->nlmsg_seq,
>  					       RTM_NEWNETCONF, NLM_F_MULTI,
>  					       NETCONFA_ALL) < 0)
>  			goto done;
> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> index 3e33934751b4..b80b00b55bdf 100644
> --- a/net/mpls/af_mpls.c
> +++ b/net/mpls/af_mpls.c
> @@ -1263,6 +1263,7 @@ static int mpls_netconf_get_devconf(struct sk_buff *in_skb,
>  static int mpls_netconf_dump_devconf(struct sk_buff *skb,
>  				     struct netlink_callback *cb)
>  {
> +	const struct nlmsghdr *nlh = cb->nlh;
>  	struct net *net = sock_net(skb->sk);
>  	struct hlist_head *head;
>  	struct net_device *dev;
> @@ -1270,6 +1271,21 @@ static int mpls_netconf_dump_devconf(struct sk_buff *skb,
>  	int idx, s_idx;
>  	int h, s_h;
>  
> +	if (cb->strict_check) {
> +		struct netlink_ext_ack *extack = cb->extack;
> +		struct netconfmsg *ncm;
> +
> +		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ncm))) {
> +			NL_SET_ERR_MSG(extack, "Invalid header");
> +			return -EINVAL;
> +		}
> +
> +		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ncm))) {
> +			NL_SET_ERR_MSG(extack, "Invalid data after header");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	s_h = cb->args[0];
>  	s_idx = idx = cb->args[1];
>  
> @@ -1286,7 +1302,7 @@ static int mpls_netconf_dump_devconf(struct sk_buff *skb,
>  				goto cont;
>  			if (mpls_netconf_fill_devconf(skb, mdev,
>  						      NETLINK_CB(cb->skb).portid,
> -						      cb->nlh->nlmsg_seq,
> +						      nlh->nlmsg_seq,
>  						      RTM_NEWNETCONF,
>  						      NLM_F_MULTI,
>  						      NETCONFA_ALL) < 0) {
> -- 
> 2.11.0
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ