| Message ID | 1439261215-28078-1-git-send-email-ira.weiny@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Aug 10, 2015 at 10:46:55PM -0400, ira.weiny@intel.com wrote: > From: Ira Weiny <ira.weiny@intel.com> > > The recently added SA Netlink service requires admin privileges to receive > kernel requests. This is only partially sufficient to protect the kernel from > malicious users. This patch fixes two issues. > > 1) Path responses from user space could be spoofed if the sequence > number was properly guessed. > 2) The set timeout request message could be issued by any user. > > Ignore these messages if not submitted by an admin user. > > Fixes: 6619209af36c ("IB/sa: Route SA pathrecord query through netlink") > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > --- > Changes from V1: > Use netlink_net_capable rather than ns_capable Doug, As per the thread with the V1 patch we are looking to merge this into a v9 of Kaikes series once we do some more testing with the netlink_bind and namespaces. So you can safely ignore both v1 and this patch. Thanks, Ira > > Doug let me know if you would prefer that I get Kaike to squash this into the > original patch. > > drivers/infiniband/core/sa_query.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c > index 70ceec4df02a..6778644a6957 100644 > --- a/drivers/infiniband/core/sa_query.c > +++ b/drivers/infiniband/core/sa_query.c > @@ -699,6 +699,12 @@ static int ib_nl_handle_set_timeout(struct sk_buff *skb, > struct nlattr *tb[LS_NLA_TYPE_MAX + 1]; > int ret; > > + if (!netlink_net_capable(skb, CAP_NET_ADMIN)) { > + pr_warn_ratelimited("SA netlink: invalid perm for set timeout: `%s'.\n", > + current->comm); > + return -EPERM; > + } > + > ret = nla_parse(tb, LS_NLA_TYPE_MAX, nlmsg_data(nlh), nlmsg_len(nlh), > NULL); > attr = (const struct nlattr *)tb[LS_NLA_TYPE_TIMEOUT]; > @@ -706,6 +712,9 @@ static int ib_nl_handle_set_timeout(struct sk_buff *skb, > goto settimeout_out; > > timeout = *(int *) nla_data(attr); > + > + pr_info("SA netlink: timeout: %d\n", timeout); > + > if (timeout < IB_SA_LOCAL_SVC_TIMEOUT_MIN) > timeout = IB_SA_LOCAL_SVC_TIMEOUT_MIN; > if (timeout > IB_SA_LOCAL_SVC_TIMEOUT_MAX) > @@ -754,6 +763,12 @@ static int ib_nl_handle_resolve_resp(struct sk_buff *skb, > int found = 0; > int ret; > > + if (!netlink_net_capable(skb, CAP_NET_ADMIN)) { > + pr_warn_ratelimited("SA netlink: invalid perm for response: `%s'.\n", > + current->comm); > + return -EPERM; > + } > + > spin_lock_irqsave(&ib_nl_request_lock, flags); > list_for_each_entry(query, &ib_nl_request_list, list) { > /* > @@ -770,6 +785,7 @@ static int ib_nl_handle_resolve_resp(struct sk_buff *skb, > > if (!found) { > spin_unlock_irqrestore(&ib_nl_request_lock, flags); > + pr_err_ratelimited("SA netlink: got unmatched response\n"); > goto resp_out; > } > > -- > 1.8.2 > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/12/2015 03:43 PM, ira.weiny wrote: > On Mon, Aug 10, 2015 at 10:46:55PM -0400, ira.weiny@intel.com wrote: >> From: Ira Weiny <ira.weiny@intel.com> >> >> The recently added SA Netlink service requires admin privileges to receive >> kernel requests. This is only partially sufficient to protect the kernel from >> malicious users. This patch fixes two issues. >> >> 1) Path responses from user space could be spoofed if the sequence >> number was properly guessed. >> 2) The set timeout request message could be issued by any user. >> >> Ignore these messages if not submitted by an admin user. >> >> Fixes: 6619209af36c ("IB/sa: Route SA pathrecord query through netlink") >> Signed-off-by: Ira Weiny <ira.weiny@intel.com> >> >> --- >> Changes from V1: >> Use netlink_net_capable rather than ns_capable > > Doug, > > As per the thread with the V1 patch we are looking to merge this into a v9 of > Kaikes series once we do some more testing with the netlink_bind and > namespaces. > > So you can safely ignore both v1 and this patch. Ok.
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c index 70ceec4df02a..6778644a6957 100644 --- a/drivers/infiniband/core/sa_query.c +++ b/drivers/infiniband/core/sa_query.c @@ -699,6 +699,12 @@ static int ib_nl_handle_set_timeout(struct sk_buff *skb, struct nlattr *tb[LS_NLA_TYPE_MAX + 1]; int ret; + if (!netlink_net_capable(skb, CAP_NET_ADMIN)) { + pr_warn_ratelimited("SA netlink: invalid perm for set timeout: `%s'.\n", + current->comm); + return -EPERM; + } + ret = nla_parse(tb, LS_NLA_TYPE_MAX, nlmsg_data(nlh), nlmsg_len(nlh), NULL); attr = (const struct nlattr *)tb[LS_NLA_TYPE_TIMEOUT]; @@ -706,6 +712,9 @@ static int ib_nl_handle_set_timeout(struct sk_buff *skb, goto settimeout_out; timeout = *(int *) nla_data(attr); + + pr_info("SA netlink: timeout: %d\n", timeout); + if (timeout < IB_SA_LOCAL_SVC_TIMEOUT_MIN) timeout = IB_SA_LOCAL_SVC_TIMEOUT_MIN; if (timeout > IB_SA_LOCAL_SVC_TIMEOUT_MAX) @@ -754,6 +763,12 @@ static int ib_nl_handle_resolve_resp(struct sk_buff *skb, int found = 0; int ret; + if (!netlink_net_capable(skb, CAP_NET_ADMIN)) { + pr_warn_ratelimited("SA netlink: invalid perm for response: `%s'.\n", + current->comm); + return -EPERM; + } + spin_lock_irqsave(&ib_nl_request_lock, flags); list_for_each_entry(query, &ib_nl_request_list, list) { /* @@ -770,6 +785,7 @@ static int ib_nl_handle_resolve_resp(struct sk_buff *skb, if (!found) { spin_unlock_irqrestore(&ib_nl_request_lock, flags); + pr_err_ratelimited("SA netlink: got unmatched response\n"); goto resp_out; }