Re: [PATCH] IB/core: Add mitigation for Spectre V1
On Tue, Jul 30, 2019 at 06:52:12PM -0500, Gustavo A. R. Silva wrote: > > > On 7/30/19 3:24 PM, Tony Luck wrote: > > Some processors may mispredict an array bounds check and > > speculatively access memory that they should not. With > > a user supplied array index we like to play things safe > > by masking the value with the array size before it is > > used as an index. > > > > Signed-off-by: Tony Luck > > --- > > > > [I don't have h/w, so just compile tested] > > > > drivers/infiniband/core/user_mad.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/infiniband/core/user_mad.c > > b/drivers/infiniband/core/user_mad.c > > index 9f8a48016b41..fdce254e4f65 100644 > > --- a/drivers/infiniband/core/user_mad.c > > +++ b/drivers/infiniband/core/user_mad.c > > @@ -49,6 +49,7 @@ > > #include > > #include > > #include > > +#include > > > > #include > > > > @@ -888,6 +889,7 @@ static int ib_umad_unreg_agent(struct ib_umad_file > > *file, u32 __user *arg) > > mutex_lock(&file->port->file_mutex); > > mutex_lock(&file->mutex); > > > > + id = array_index_nospec(id, IB_UMAD_MAX_AGENTS); > > This is wrong. This prevents the below condition id >= IB_UMAD_MAX_AGENTS > from ever being true. And I don't think this is what you want. Ah Yea... FWIW this would probably never be hit. Tony; split the check? if (id >= IB_UMAD_MAX_AGENTS) { ret = -EINVAL; goto out; } id = array_index_nospec(id, IB_UMAD_MAX_AGENTS); if (!__get_agent(file, id)) { ret = -EINVAL; goto out; } Ira > > > if (id >= IB_UMAD_MAX_AGENTS || !__get_agent(file, id)) { > > ret = -EINVAL; > > goto out; > > > > -- > Gustavo
Re: [PATCH] IB/core: Add mitigation for Spectre V1
On 7/30/19 3:24 PM, Tony Luck wrote: > Some processors may mispredict an array bounds check and > speculatively access memory that they should not. With > a user supplied array index we like to play things safe > by masking the value with the array size before it is > used as an index. > > Signed-off-by: Tony Luck > --- > > [I don't have h/w, so just compile tested] > > drivers/infiniband/core/user_mad.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/infiniband/core/user_mad.c > b/drivers/infiniband/core/user_mad.c > index 9f8a48016b41..fdce254e4f65 100644 > --- a/drivers/infiniband/core/user_mad.c > +++ b/drivers/infiniband/core/user_mad.c > @@ -49,6 +49,7 @@ > #include > #include > #include > +#include > > #include > > @@ -888,6 +889,7 @@ static int ib_umad_unreg_agent(struct ib_umad_file *file, > u32 __user *arg) > mutex_lock(&file->port->file_mutex); > mutex_lock(&file->mutex); > > + id = array_index_nospec(id, IB_UMAD_MAX_AGENTS); This is wrong. This prevents the below condition id >= IB_UMAD_MAX_AGENTS from ever being true. And I don't think this is what you want. > if (id >= IB_UMAD_MAX_AGENTS || !__get_agent(file, id)) { > ret = -EINVAL; > goto out; > -- Gustavo
Re: [PATCH] IB/core: Add mitigation for Spectre V1
On Tue, Jul 30, 2019 at 01:24:07PM -0700, Tony Luck wrote: > Some processors may mispredict an array bounds check and > speculatively access memory that they should not. With > a user supplied array index we like to play things safe > by masking the value with the array size before it is > used as an index. > > Signed-off-by: Tony Luck Reviewed-by: Ira Weiny Tested-by: Ira Weiny > --- > > [I don't have h/w, so just compile tested] > > drivers/infiniband/core/user_mad.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/infiniband/core/user_mad.c > b/drivers/infiniband/core/user_mad.c > index 9f8a48016b41..fdce254e4f65 100644 > --- a/drivers/infiniband/core/user_mad.c > +++ b/drivers/infiniband/core/user_mad.c > @@ -49,6 +49,7 @@ > #include > #include > #include > +#include > > #include > > @@ -888,6 +889,7 @@ static int ib_umad_unreg_agent(struct ib_umad_file *file, > u32 __user *arg) > mutex_lock(&file->port->file_mutex); > mutex_lock(&file->mutex); > > + id = array_index_nospec(id, IB_UMAD_MAX_AGENTS); > if (id >= IB_UMAD_MAX_AGENTS || !__get_agent(file, id)) { > ret = -EINVAL; > goto out; > -- > 2.20.1 >
[PATCH] IB/core: Add mitigation for Spectre V1
Some processors may mispredict an array bounds check and speculatively access memory that they should not. With a user supplied array index we like to play things safe by masking the value with the array size before it is used as an index. Signed-off-by: Tony Luck --- [I don't have h/w, so just compile tested] drivers/infiniband/core/user_mad.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c index 9f8a48016b41..fdce254e4f65 100644 --- a/drivers/infiniband/core/user_mad.c +++ b/drivers/infiniband/core/user_mad.c @@ -49,6 +49,7 @@ #include #include #include +#include #include @@ -888,6 +889,7 @@ static int ib_umad_unreg_agent(struct ib_umad_file *file, u32 __user *arg) mutex_lock(&file->port->file_mutex); mutex_lock(&file->mutex); + id = array_index_nospec(id, IB_UMAD_MAX_AGENTS); if (id >= IB_UMAD_MAX_AGENTS || !__get_agent(file, id)) { ret = -EINVAL; goto out; -- 2.20.1