| Message ID | 02bf2ebafbc5d757c0d250d8e5fba3ebdc7393fd.1463590718.git.dledford@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, May 18, 2016 at 12:58:38PM -0400, Doug Ledford wrote: > From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > The drivers/infiniband stack uses write() as a replacement for > bi-directional ioctl(). This is not safe. There are ways to > trigger write calls that result in the return structure that > is normally written to user space being shunted off to user > specified kernel memory instead. > > For the immediate repair, detect and deny suspicious accesses to > the write API. > > For long term, update the user space libraries and the kernel API > to something that doesn't present the same security vulnerabilities > (likely a structured ioctl() interface). > > The impacted uAPI interfaces are generally only available if > hardware from drivers/infiniband is installed in the system. > > Reported-by: Jann Horn <jann@thejh.net> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > [ Expanded check to all known write() entry points ] > Cc: stable@vger.kernel.org # 3.14.x > Signed-off-by: Doug Ledford <dledford@redhat.com> > [ Expanded to include removed ipath driver, and dropped non-existent > hfi1 driver ] > --- > drivers/infiniband/core/ucm.c | 4 ++++ > drivers/infiniband/core/ucma.c | 3 +++ > drivers/infiniband/core/uverbs_main.c | 5 +++++ > drivers/infiniband/hw/ipath/ipath_file_ops.c | 5 +++++ > drivers/infiniband/hw/qib/qib_file_ops.c | 5 +++++ > include/rdma/ib.h | 16 ++++++++++++++++ > 6 files changed, 38 insertions(+) I don't understand, is this only for 3.14.x? If so, what is the git commit id in Linus's tree for this? thanks, greg k-h -- 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 05/18/2016 01:20 PM, Greg KH wrote: > On Wed, May 18, 2016 at 12:58:38PM -0400, Doug Ledford wrote: >> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> >> The drivers/infiniband stack uses write() as a replacement for >> bi-directional ioctl(). This is not safe. There are ways to >> trigger write calls that result in the return structure that >> is normally written to user space being shunted off to user >> specified kernel memory instead. >> >> For the immediate repair, detect and deny suspicious accesses to >> the write API. >> >> For long term, update the user space libraries and the kernel API >> to something that doesn't present the same security vulnerabilities >> (likely a structured ioctl() interface). >> >> The impacted uAPI interfaces are generally only available if >> hardware from drivers/infiniband is installed in the system. >> >> Reported-by: Jann Horn <jann@thejh.net> >> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> >> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> [ Expanded check to all known write() entry points ] >> Cc: stable@vger.kernel.org # 3.14.x >> Signed-off-by: Doug Ledford <dledford@redhat.com> >> [ Expanded to include removed ipath driver, and dropped non-existent >> hfi1 driver ] >> --- >> drivers/infiniband/core/ucm.c | 4 ++++ >> drivers/infiniband/core/ucma.c | 3 +++ >> drivers/infiniband/core/uverbs_main.c | 5 +++++ >> drivers/infiniband/hw/ipath/ipath_file_ops.c | 5 +++++ >> drivers/infiniband/hw/qib/qib_file_ops.c | 5 +++++ >> include/rdma/ib.h | 16 ++++++++++++++++ >> 6 files changed, 38 insertions(+) > > I don't understand, is this only for 3.14.x? Yes. You already applied the same patch against 4.5.x, but without the update to include the ipath driver and with the hfi1 driver still in the patch. > If so, what is the git > commit id in Linus's tree for this? Sorry, forgot that: e6bd18f57aad
On Wed, May 18, 2016 at 8:57 PM, Doug Ledford <dledford@redhat.com> wrote: > On 05/18/2016 01:20 PM, Greg KH wrote: >> I don't understand, is this only for 3.14.x? > Yes. Wait, any reason not to apply it on all stable kernels? Or. > You already applied the same patch against 4.5.x, but without the > update to include the ipath driver and with the hfi1 driver still in the patch. -- 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 05/18/2016 02:59 PM, Or Gerlitz wrote: > On Wed, May 18, 2016 at 8:57 PM, Doug Ledford <dledford@redhat.com> wrote: >> On 05/18/2016 01:20 PM, Greg KH wrote: > >>> I don't understand, is this only for 3.14.x? > >> Yes. > > Wait, any reason not to apply it on all stable kernels? It already is. The only kernel that Greg takes care of that it failed to apply to was this one, and so nothing was applied. For the two kernels that Kamal manages, he munged the upstream patch to apply by dropping the hfi1 hunks, but that didn't add in the missing ipath hunks, so that's the reason for the two other patches I sent, they were for him so that he could apply those to his two trees to update his patch to cover everything. > Or. > >> You already applied the same patch against 4.5.x, but without the >> update to include the ipath driver and with the hfi1 driver still in the patch.
On Wed, May 18, 2016 at 10:05 PM, Doug Ledford <dledford@redhat.com> wrote: > On 05/18/2016 02:59 PM, Or Gerlitz wrote: >> On Wed, May 18, 2016 at 8:57 PM, Doug Ledford <dledford@redhat.com> wrote: >>> On 05/18/2016 01:20 PM, Greg KH wrote: >> >>>> I don't understand, is this only for 3.14.x? >> >>> Yes. >> >> Wait, any reason not to apply it on all stable kernels? > > It already is. Is there a special reason you didn't add the CVE number to the change-log? Or. -- 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 05/18/2016 03:47 PM, Or Gerlitz wrote: > On Wed, May 18, 2016 at 10:05 PM, Doug Ledford <dledford@redhat.com> wrote: >> On 05/18/2016 02:59 PM, Or Gerlitz wrote: >>> On Wed, May 18, 2016 at 8:57 PM, Doug Ledford <dledford@redhat.com> wrote: >>>> On 05/18/2016 01:20 PM, Greg KH wrote: >>> >>>>> I don't understand, is this only for 3.14.x? >>> >>>> Yes. >>> >>> Wait, any reason not to apply it on all stable kernels? >> >> It already is. > > Is there a special reason you didn't add the CVE number to the change-log? I didn't write the commit change log. I used the commit change log from the original patch that didn't fail to apply.
On Wed, May 18, 2016 at 10:59 PM, Doug Ledford <dledford@redhat.com> wrote: > On 05/18/2016 03:47 PM, Or Gerlitz wrote: >> Is there a special reason you didn't add the CVE number to the change-log? > I didn't write the commit change log. I used the commit change log from > the original patch that didn't fail to apply. I mean on the original patch when you added your maintainer S.O.B Or. -- 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 05/18/2016 04:36 PM, Or Gerlitz wrote: > On Wed, May 18, 2016 at 10:59 PM, Doug Ledford <dledford@redhat.com> wrote: >> On 05/18/2016 03:47 PM, Or Gerlitz wrote: > >>> Is there a special reason you didn't add the CVE number to the change-log? > >> I didn't write the commit change log. I used the commit change log from >> the original patch that didn't fail to apply. > > I mean on the original patch when you added your maintainer S.O.B Because of an issue of confusion. Red Hat had an internal CVE assigned, but I didn't know I could put that on here. There wasn't an assigned MITRE CVE yet. Then people noticed the patch, someone requested a CVE on the mitre list, and another one got assigned. Red Hat has since dropped their internal CVE and adopted the mitre CVE. That probably would have been avoided had I known to put Red Hat's internal CVE on there.
diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c index f2f63933e8a9..5befec118a18 100644 --- a/drivers/infiniband/core/ucm.c +++ b/drivers/infiniband/core/ucm.c @@ -48,6 +48,7 @@ #include <asm/uaccess.h> +#include <rdma/ib.h> #include <rdma/ib_cm.h> #include <rdma/ib_user_cm.h> #include <rdma/ib_marshall.h> @@ -1104,6 +1105,9 @@ static ssize_t ib_ucm_write(struct file *filp, const char __user *buf, struct ib_ucm_cmd_hdr hdr; ssize_t result; + if (WARN_ON_ONCE(!ib_safe_file_access(filp))) + return -EACCES; + if (len < sizeof(hdr)) return -EINVAL; diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c index 56a4b7ca7ee3..22e02805147b 100644 --- a/drivers/infiniband/core/ucma.c +++ b/drivers/infiniband/core/ucma.c @@ -1484,6 +1484,9 @@ static ssize_t ucma_write(struct file *filp, const char __user *buf, struct rdma_ucm_cmd_hdr hdr; ssize_t ret; + if (WARN_ON_ONCE(!ib_safe_file_access(filp))) + return -EACCES; + if (len < sizeof(hdr)) return -EINVAL; diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c index 08219fb3338b..d12ae04657c8 100644 --- a/drivers/infiniband/core/uverbs_main.c +++ b/drivers/infiniband/core/uverbs_main.c @@ -48,6 +48,8 @@ #include <asm/uaccess.h> +#include <rdma/ib.h> + #include "uverbs.h" MODULE_AUTHOR("Roland Dreier"); @@ -594,6 +596,9 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf, struct ib_uverbs_cmd_hdr hdr; __u32 flags; + if (WARN_ON_ONCE(!ib_safe_file_access(filp))) + return -EACCES; + if (count < sizeof hdr) return -EINVAL; diff --git a/drivers/infiniband/hw/ipath/ipath_file_ops.c b/drivers/infiniband/hw/ipath/ipath_file_ops.c index 6d7f453b4d05..2b828ea3ac4a 100644 --- a/drivers/infiniband/hw/ipath/ipath_file_ops.c +++ b/drivers/infiniband/hw/ipath/ipath_file_ops.c @@ -45,6 +45,8 @@ #include <linux/cpu.h> #include <asm/pgtable.h> +#include "rdma/ib.h" + #include "ipath_kernel.h" #include "ipath_common.h" #include "ipath_user_sdma.h" @@ -2240,6 +2242,9 @@ static ssize_t ipath_write(struct file *fp, const char __user *data, ssize_t ret = 0; void *dest; + if (WARN_ON_ONCE(!ib_safe_file_access(filp))) + return -EACCES; + if (count < sizeof(cmd.type)) { ret = -EINVAL; goto bail; diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c index 275f247f9fca..ff5e7acc8d3f 100644 --- a/drivers/infiniband/hw/qib/qib_file_ops.c +++ b/drivers/infiniband/hw/qib/qib_file_ops.c @@ -45,6 +45,8 @@ #include <linux/delay.h> #include <linux/export.h> +#include <rdma/ib.h> + #include "qib.h" #include "qib_common.h" #include "qib_user_sdma.h" @@ -2057,6 +2059,9 @@ static ssize_t qib_write(struct file *fp, const char __user *data, ssize_t ret = 0; void *dest; + if (WARN_ON_ONCE(!ib_safe_file_access(fp))) + return -EACCES; + if (count < sizeof(cmd.type)) { ret = -EINVAL; goto bail; diff --git a/include/rdma/ib.h b/include/rdma/ib.h index cf8f9e700e48..a6b93706b0fc 100644 --- a/include/rdma/ib.h +++ b/include/rdma/ib.h @@ -34,6 +34,7 @@ #define _RDMA_IB_H #include <linux/types.h> +#include <linux/sched.h> struct ib_addr { union { @@ -86,4 +87,19 @@ struct sockaddr_ib { __u64 sib_scope_id; }; +/* + * The IB interfaces that use write() as bi-directional ioctl() are + * fundamentally unsafe, since there are lots of ways to trigger "write()" + * calls from various contexts with elevated privileges. That includes the + * traditional suid executable error message writes, but also various kernel + * interfaces that can write to file descriptors. + * + * This function provides protection for the legacy API by restricting the + * calling context. + */ +static inline bool ib_safe_file_access(struct file *filp) +{ + return filp->f_cred == current_cred() && segment_eq(get_fs(), USER_DS); +} + #endif /* _RDMA_IB_H */