* [PATCH] IB/hfi1: Avoid hardlockup with flushlist_lock
From: Mike Marciniszyn @ 2019-06-24 16:26 UTC (permalink / raw)
To: stable; +Cc: linux-rdma, stable-commits
commit cf131a81967583ae737df6383a0893b9fee75b4e upstream.
Heavy contention of the sde flushlist_lock can cause hard lockups at
extreme scale when the flushing logic is under stress.
Mitigate by replacing the item at a time copy to the local list with
an O(1) list_splice_init() and using the high priority work queue to
do the flushes.
Ported to linux-4.19.y.
Fixes: 7724105686e7 ("IB/hfi1: add driver files")
Cc: <stable@vger.kernel.org>
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>
---
drivers/infiniband/hw/hfi1/sdma.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/sdma.c b/drivers/infiniband/hw/hfi1/sdma.c
index 88e326d..d648a41 100644
--- a/drivers/infiniband/hw/hfi1/sdma.c
+++ b/drivers/infiniband/hw/hfi1/sdma.c
@@ -410,10 +410,7 @@ static void sdma_flush(struct sdma_engine *sde)
sdma_flush_descq(sde);
spin_lock_irqsave(&sde->flushlist_lock, flags);
/* copy flush list */
- list_for_each_entry_safe(txp, txp_next, &sde->flushlist, list) {
- list_del_init(&txp->list);
- list_add_tail(&txp->list, &flushlist);
- }
+ list_splice_init(&sde->flushlist, &flushlist);
spin_unlock_irqrestore(&sde->flushlist_lock, flags);
/* flush from flush list */
list_for_each_entry_safe(txp, txp_next, &flushlist, list)
@@ -2426,7 +2423,7 @@ int sdma_send_txreq(struct sdma_engine *sde,
wait->tx_count++;
wait->count += tx->num_desc;
}
- schedule_work(&sde->flush_worker);
+ queue_work_on(sde->cpu, system_highpri_wq, &sde->flush_worker);
ret = -ECOMM;
goto unlock;
nodesc:
@@ -2526,7 +2523,7 @@ int sdma_send_txlist(struct sdma_engine *sde, struct iowait *wait,
}
}
spin_unlock(&sde->flushlist_lock);
- schedule_work(&sde->flush_worker);
+ queue_work_on(sde->cpu, system_highpri_wq, &sde->flush_worker);
ret = -ECOMM;
goto update_tail;
nodesc:
^ permalink raw reply related
* [PATCH] IB/hfi1: Avoid hardlockup with flushlist_lock
From: Mike Marciniszyn @ 2019-06-24 16:14 UTC (permalink / raw)
To: stable; +Cc: linux-rdma, stable-commits
commit cf131a81967583ae737df6383a0893b9fee75b4e upstream.
Heavy contention of the sde flushlist_lock can cause hard lockups at
extreme scale when the flushing logic is under stress.
Mitigate by replacing the item at a time copy to the local list with
an O(1) list_splice_init() and using the high priority work queue to
do the flushes.
Ported to linux-4.14.y.
Fixes: 7724105686e7 ("IB/hfi1: add driver files")
Cc: <stable@vger.kernel.org>
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>
---
drivers/infiniband/hw/hfi1/sdma.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/sdma.c b/drivers/infiniband/hw/hfi1/sdma.c
index 6781bcd..ec613d5 100644
--- a/drivers/infiniband/hw/hfi1/sdma.c
+++ b/drivers/infiniband/hw/hfi1/sdma.c
@@ -410,10 +410,7 @@ static void sdma_flush(struct sdma_engine *sde)
sdma_flush_descq(sde);
spin_lock_irqsave(&sde->flushlist_lock, flags);
/* copy flush list */
- list_for_each_entry_safe(txp, txp_next, &sde->flushlist, list) {
- list_del_init(&txp->list);
- list_add_tail(&txp->list, &flushlist);
- }
+ list_splice_init(&sde->flushlist, &flushlist);
spin_unlock_irqrestore(&sde->flushlist_lock, flags);
/* flush from flush list */
list_for_each_entry_safe(txp, txp_next, &flushlist, list)
@@ -2437,7 +2434,7 @@ int sdma_send_txreq(struct sdma_engine *sde,
wait->tx_count++;
wait->count += tx->num_desc;
}
- schedule_work(&sde->flush_worker);
+ queue_work_on(sde->cpu, system_highpri_wq, &sde->flush_worker);
ret = -ECOMM;
goto unlock;
nodesc:
@@ -2537,7 +2534,7 @@ int sdma_send_txlist(struct sdma_engine *sde, struct iowait *wait,
}
}
spin_unlock(&sde->flushlist_lock);
- schedule_work(&sde->flush_worker);
+ queue_work_on(sde->cpu, system_highpri_wq, &sde->flush_worker);
ret = -ECOMM;
goto update_tail;
nodesc:
^ permalink raw reply related
* Re: [PATCH rdma-next v1 10/12] IB/mlx5: Enable subscription for device events over DEVX
From: Yishai Hadas @ 2019-06-24 16:13 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky, RDMA mailing list,
Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <20190624115726.GC5479@mellanox.com>
On 6/24/2019 2:57 PM, Jason Gunthorpe wrote:
> On Tue, Jun 18, 2019 at 08:15:38PM +0300, Leon Romanovsky wrote:
>> From: Yishai Hadas <yishaih@mellanox.com>
>>
>> Enable subscription for device events over DEVX.
>>
>> Each subscription is added to the two level XA data structure according
>> to its event number and the DEVX object information in case was given
>> with the given target fd.
>>
>> Those events will be reported over the given fd once will occur.
>> Downstream patches will mange the dispatching to any subscription.
>>
>> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
>> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>> drivers/infiniband/hw/mlx5/devx.c | 564 ++++++++++++++++++++++-
>> include/uapi/rdma/mlx5_user_ioctl_cmds.h | 9 +
>> 2 files changed, 566 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
>> index e9b9ba5a3e9a..304b13e7a265 100644
>> +++ b/drivers/infiniband/hw/mlx5/devx.c
>> @@ -14,6 +14,7 @@
>> #include <linux/mlx5/driver.h>
>> #include <linux/mlx5/fs.h>
>> #include "mlx5_ib.h"
>> +#include <linux/xarray.h>
>>
>> #define UVERBS_MODULE_NAME mlx5_ib
>> #include <rdma/uverbs_named_ioctl.h>
>> @@ -33,6 +34,37 @@ struct devx_async_data {
>> struct mlx5_ib_uapi_devx_async_cmd_hdr hdr;
>> };
>>
>> +/* first level XA value data structure */
>> +struct devx_event {
>> + struct xarray object_ids; /* second XA level, Key = object id */
>> + struct list_head unaffiliated_list;
>> +};
>> +
>> +/* second level XA value data structure */
>> +struct devx_obj_event {
>> + struct rcu_head rcu;
>> + struct list_head obj_sub_list;
>> +};
>> +
>> +struct devx_event_subscription {
>> + struct list_head file_list; /* headed in private_data->
>> + * subscribed_events_list
>> + */
>> + struct list_head xa_list; /* headed in devx_event->unaffiliated_list or
>> + * devx_obj_event->obj_sub_list
>> + */
>> + struct list_head obj_list; /* headed in devx_object */
>> +
>> + u32 xa_key_level1;
>> + u32 xa_key_level2;
>> + struct rcu_head rcu;
>> + u64 cookie;
>> + bool is_obj_related;
>> + struct ib_uobject *fd_uobj;
>> + void *object; /* May need direct access upon hot unplug */
>
> This should be a 'struct file *' and have a better name.
>
OK, will change.
> And I'm unclear why we need to store both the ib_uobject and the
> struct file for the same thing?
Post hot unplug/unbind the uobj can't be accessed any more to reach the
object as it will be set to NULL by ib_core layer [1].
As the filp is still open we need a direct access to it down the road
and for that we have it separately.
This was the comment that I have just put above in the code, I may
improve it with more details as pointed here.
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/infiniband/core/rdma_core.c#L149
And why are we storing the uobj here
> instead of the struct devx_async_event_file *?
>
Basically storing the uobj is the same as of storing the
devx_async_event_file, as we just use container_of to get it from.
There is no direct access from uobj to any of its fields so that we
should be fine.
> Since uobj->object == flip && filp->private_data == uobj, I have a
> hard time to understand why we need both things, it seems to me that
> if we get the fget on the filp then we can rely on the
> filp->private_data to get back to the devx_async_event_file.
>
The idea was to not take an extra ref count on the file (i.e. fget) per
subscription, this will let the release option to be called once the
file will be closed by the application.
Otherwise we might need to consider having some unsubscribe method to
put the ref count back with all its overhead and implications without a
real justified reason.
>> + struct eventfd_ctx *eventfd;
>> +};
>> +
>
>> /*
>> * As the obj_id in the firmware is not globally unique the object type
>> * must be considered upon checking for a valid object id.
>> @@ -1143,14 +1275,53 @@ static void devx_cleanup_mkey(struct devx_obj *obj)
>> write_unlock_irqrestore(&table->lock, flags);
>> }
>>
>> +static void devx_cleanup_subscription(struct mlx5_ib_dev *dev,
>> + struct devx_event_subscription *sub)
>> +{
>> + list_del_rcu(&sub->file_list);
>> + list_del_rcu(&sub->xa_list);
>> +
>> + if (sub->is_obj_related) {
>
> is_obj_related looks like it is just list_empty(obj_list)??
>
Yes, in that approach we may need to call INIT_LIST_HEAD(sub->obj_list)
in case it wasn't an object upon subscription, will do that.
> Success oriented flow
>
>> @@ -1523,6 +1700,350 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_ASYNC_QUERY)(
>> return err;
>> }
>>
>> +static void
>> +subscribe_event_xa_dealloc(struct mlx5_devx_event_table *devx_event_table,
>> + u32 key_level1,
>> + u32 key_level2,
>> + struct devx_obj_event *alloc_obj_event)
>> +{
>> + struct devx_event *event;
>> +
>> + /* Level 1 is valid for future use - no need to free */
>> + if (!alloc_obj_event)
>> + return;
>> +
>> + event = xa_load(&devx_event_table->event_xa, key_level1);
>> + WARN_ON(!event);
>> +
>> + xa_erase(&event->object_ids, key_level2);
>
> Shoulnd't this only erase if the value stored is NULL?
>
If this key_level2 wasn't allocated by the subscribe flow and exists
before we may not reach here and return from the above lines in this
function [1], otherwise we need to erase as done here.
[1]
if (!alloc_obj_event)
return;
>> + kfree(alloc_obj_event);
>> +}
>> +
>> +static int
>> +subscribe_event_xa_alloc(struct mlx5_devx_event_table *devx_event_table,
>> + u32 key_level1,
>> + bool is_level2,
>> + u32 key_level2,
>> + struct devx_obj_event **alloc_obj_event)
>> +{
>> + struct devx_obj_event *obj_event;
>> + struct devx_event *event;
>> + bool new_entry_level1 = false;
>> + int err;
>> +
>> + event = xa_load(&devx_event_table->event_xa, key_level1);
>> + if (!event) {
>> + event = kzalloc(sizeof(*event), GFP_KERNEL);
>> + if (!event)
>> + return -ENOMEM;
>> +
>> + new_entry_level1 = true;
>> + INIT_LIST_HEAD(&event->unaffiliated_list);
>> + xa_init(&event->object_ids);
>> +
>> + err = xa_insert(&devx_event_table->event_xa,
>> + key_level1,
>> + event,
>> + GFP_KERNEL);
>> + if (err)
>> + goto end;
>> + }
>> +
>> + if (!is_level2)
>> + return 0;
>> +
>> + obj_event = xa_load(&event->object_ids, key_level2);
>> + if (!obj_event) {
>> + err = xa_reserve(&event->object_ids, key_level2, GFP_KERNEL);
>> + if (err)
>> + goto err_level1;
>> +
>> + obj_event = kzalloc(sizeof(*obj_event), GFP_KERNEL);
>> + if (!obj_event) {
>> + err = -ENOMEM;
>> + goto err_level2;
>> + }
>> +
>> + INIT_LIST_HEAD(&obj_event->obj_sub_list);
>> + *alloc_obj_event = obj_event;
>
> This is goofy, just store the empty obj_event in the xa instead of
> using xa_reserve, and when you go to do the error unwind just delete
> any level2' devx_obj_event' that has a list_empty(obj_sub_list), get
> rid of the wonky alloc_obj_event stuff.
>
Please see my answer above about how level2 is managed by this
alloc_obj_event, is that really worth a change ? I found current logic
to be clear. I may put some note here if we can stay with that.
> The best configuration would be to use devx_cleanup_subscription to
> undo the partially ready subscription.
>
This partially ready subscription might not match the
devx_cleanup_subscription(), e.g. it wasn't added to xa_list and can't
be deleted without any specific flag to ignore ..
>> + }
>> +
>> + return 0;
>> +
>> +err_level2:
>> + xa_erase(&event->object_ids, key_level2);
>> +
>> +err_level1:
>> + if (new_entry_level1)
>> + xa_erase(&devx_event_table->event_xa, key_level1);
>> +end:
>> + if (new_entry_level1)
>> + kfree(event);
>
> Can't do this, once the level1 is put in the tree it could be referenced by
> the irqs. At least it needs a kfree_rcu, most likely it is simpler to
> just leave it.
>
Agree, it looks simpler just to leave it, will handle.
>> +#define MAX_NUM_EVENTS 16
>> +static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT)(
>> + struct uverbs_attr_bundle *attrs)
>> +{
>> + struct ib_uobject *devx_uobj = uverbs_attr_get_uobject(
>> + attrs,
>> + MLX5_IB_ATTR_DEVX_SUBSCRIBE_EVENT_OBJ_HANDLE);
>> + struct mlx5_ib_ucontext *c = rdma_udata_to_drv_context(
>> + &attrs->driver_udata, struct mlx5_ib_ucontext, ibucontext);
>> + struct mlx5_ib_dev *dev = to_mdev(c->ibucontext.device);
>> + struct ib_uobject *fd_uobj;
>> + struct devx_obj *obj = NULL;
>> + struct devx_async_event_file *ev_file;
>> + struct mlx5_devx_event_table *devx_event_table = &dev->devx_event_table;
>> + u16 *event_type_num_list;
>> + struct devx_event_subscription **event_sub_arr;
>> + struct devx_obj_event **event_obj_array_alloc;
>> + int redirect_fd;
>> + bool use_eventfd = false;
>> + int num_events;
>> + int num_alloc_xa_entries = 0;
>> + u16 obj_type = 0;
>> + u64 cookie = 0;
>> + u32 obj_id = 0;
>> + int err;
>> + int i;
>> +
>> + if (!c->devx_uid)
>> + return -EINVAL;
>> +
>> + if (!IS_ERR(devx_uobj)) {
>> + obj = (struct devx_obj *)devx_uobj->object;
>> + if (obj)
>> + obj_id = get_dec_obj_id(obj->obj_id);
>> + }
>> +
>> + fd_uobj = uverbs_attr_get_uobject(attrs,
>> + MLX5_IB_ATTR_DEVX_SUBSCRIBE_EVENT_FD_HANDLE);
>> + if (IS_ERR(fd_uobj))
>> + return PTR_ERR(fd_uobj);
>> +
>> + ev_file = container_of(fd_uobj, struct devx_async_event_file,
>> + uobj);
>> +
>> + if (uverbs_attr_is_valid(attrs,
>> + MLX5_IB_ATTR_DEVX_SUBSCRIBE_EVENT_FD_NUM)) {
>> + err = uverbs_copy_from(&redirect_fd, attrs,
>> + MLX5_IB_ATTR_DEVX_SUBSCRIBE_EVENT_FD_NUM);
>> + if (err)
>> + return err;
>> +
>> + use_eventfd = true;
>> + }
>> +
>> + if (uverbs_attr_is_valid(attrs,
>> + MLX5_IB_ATTR_DEVX_SUBSCRIBE_EVENT_COOKIE)) {
>> + if (use_eventfd)
>> + return -EINVAL;
>> +
>> + err = uverbs_copy_from(&cookie, attrs,
>> + MLX5_IB_ATTR_DEVX_SUBSCRIBE_EVENT_COOKIE);
>> + if (err)
>> + return err;
>> + }
>> +
>> + num_events = uverbs_attr_ptr_get_array_size(
>> + attrs, MLX5_IB_ATTR_DEVX_SUBSCRIBE_EVENT_TYPE_NUM_LIST,
>> + sizeof(u16));
>> +
>> + if (num_events < 0)
>> + return num_events;
>> +
>> + if (num_events > MAX_NUM_EVENTS)
>> + return -EINVAL;
>> +
>> + event_type_num_list = uverbs_attr_get_alloced_ptr(attrs,
>> + MLX5_IB_ATTR_DEVX_SUBSCRIBE_EVENT_TYPE_NUM_LIST);
>> +
>> + if (!is_valid_events(dev->mdev, num_events, event_type_num_list, obj))
>> + return -EINVAL;
>> +
>> + event_sub_arr = uverbs_zalloc(attrs,
>> + MAX_NUM_EVENTS * sizeof(struct devx_event_subscription *));
>> + event_obj_array_alloc = uverbs_zalloc(attrs,
>> + MAX_NUM_EVENTS * sizeof(struct devx_obj_event *));
>
> There are so many list_heads in the devx_event_subscription, why not
> use just one of them to store the allocated events instead of this
> temp array? ie event_list looks good for this purpose.
>
I'm using the array later on with direct access to the index that should
be de-allocated. I would prefer staying with this array rather than
using the 'event_list' which has other purpose down the road, it's used
per subscription and doesn't look match to hold the devx_obj_event which
has no list entry for this purpose..
>> +
>> + if (!event_sub_arr || !event_obj_array_alloc)
>> + return -ENOMEM;
>> +
>> + /* Protect from concurrent subscriptions to same XA entries to allow
>> + * both to succeed
>> + */
>> + mutex_lock(&devx_event_table->event_xa_lock);
>> + for (i = 0; i < num_events; i++) {
>> + u32 key_level1;
>> +
>> + if (obj)
>> + obj_type = get_dec_obj_type(obj,
>> + event_type_num_list[i]);
>> + key_level1 = event_type_num_list[i] | obj_type << 16;
>> +
>> + err = subscribe_event_xa_alloc(devx_event_table,
>> + key_level1,
>> + obj ? true : false,
>> + obj_id,
>> + &event_obj_array_alloc[i]);
>
> Usless ?:
What do you suggest instead ?
>
>> + if (err)
>> + goto err;
>> +
>> + num_alloc_xa_entries++;
>> + event_sub_arr[i] = kzalloc(sizeof(*event_sub_arr[i]),
>> + GFP_KERNEL);
>> + if (!event_sub_arr[i])
>> + goto err;
>> +
>> + if (use_eventfd) {
>> + event_sub_arr[i]->eventfd =
>> + eventfd_ctx_fdget(redirect_fd);
>> +
>> + if (IS_ERR(event_sub_arr[i]->eventfd)) {
>> + err = PTR_ERR(event_sub_arr[i]->eventfd);
>> + event_sub_arr[i]->eventfd = NULL;
>> + goto err;
>> + }
>> + }
>> +
>> + event_sub_arr[i]->cookie = cookie;
>> + event_sub_arr[i]->fd_uobj = fd_uobj;
>> + event_sub_arr[i]->object = fd_uobj->object;
>> + /* May be needed upon cleanup the devx object/subscription */
>> + event_sub_arr[i]->xa_key_level1 = key_level1;
>> + event_sub_arr[i]->xa_key_level2 = obj_id;
>> + event_sub_arr[i]->is_obj_related = obj ? true : false;
>
^ permalink raw reply
* Re: [RFC PATCH 00/28] Removing struct page from P2PDMA
From: Logan Gunthorpe @ 2019-06-24 16:10 UTC (permalink / raw)
To: Jason Gunthorpe, Christoph Hellwig
Cc: Dan Williams, Linux Kernel Mailing List, linux-block, linux-nvme,
linux-pci, linux-rdma, Jens Axboe, Bjorn Helgaas, Sagi Grimberg,
Keith Busch, Stephen Bates
In-Reply-To: <20190624134641.GA8268@ziepe.ca>
On 2019-06-24 7:46 a.m., Jason Gunthorpe wrote:
> On Mon, Jun 24, 2019 at 09:31:26AM +0200, Christoph Hellwig wrote:
>> On Thu, Jun 20, 2019 at 04:33:53PM -0300, Jason Gunthorpe wrote:
>>>> My primary concern with this is that ascribes a level of generality
>>>> that just isn't there for peer-to-peer dma operations. "Peer"
>>>> addresses are not "DMA" addresses, and the rules about what can and
>>>> can't do peer-DMA are not generically known to the block layer.
>>>
>>> ?? The P2P infrastructure produces a DMA bus address for the
>>> initiating device that is is absolutely a DMA address. There is some
>>> intermediate CPU centric representation, but after mapping it is the
>>> same as any other DMA bus address.
>>>
>>> The map function can tell if the device pair combination can do p2p or
>>> not.
>>
>> At the PCIe level there is no such thing as a DMA address, it all
>> is bus address with MMIO and DMA in the same address space (without
>> that P2P would have not chance of actually working obviously). But
>> that bus address space is different per "bus" (which would be an
>> root port in PCIe), and we need to be careful about that.
>
> Sure, that is how dma_addr_t is supposed to work - it is always a
> device specific value that can be used only by the device that it was
> created for, and different devices could have different dma_addr_t
> values for the same memory.
>
> So when Logan goes and puts dma_addr_t into the block stack he must
> also invert things so that the DMA map happens at the start of the
> process to create the right dma_addr_t early.
Yes, that's correct. The intent was to invert it so the dma_map could
happen at the start of the process so that P2PDMA code could be called
with all the information it needs to make it's decision on how to map;
without having to hook into the mapping process of every driver that
wants to participate.
Logan
^ permalink raw reply
* Re: [RFC PATCH 00/28] Removing struct page from P2PDMA
From: Logan Gunthorpe @ 2019-06-24 16:07 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-kernel, linux-block, linux-nvme, linux-pci, linux-rdma,
Jens Axboe, Bjorn Helgaas, Dan Williams, Sagi Grimberg,
Keith Busch, Jason Gunthorpe, Stephen Bates
In-Reply-To: <20190624072752.GA3954@lst.de>
On 2019-06-24 1:27 a.m., Christoph Hellwig wrote:
> This is not going to fly.
>
> For one passing a dma_addr_t through the block layer is a layering
> violation, and one that I think will also bite us in practice.
> The host physical to PCIe bus address mapping can have offsets, and
> those offsets absolutely can be different for differnet root ports.
> So with your caller generated dma_addr_t everything works fine with
> a switched setup as the one you are probably testing on, but on a
> sufficiently complicated setup with multiple root ports it can break.
I don't follow this argument. Yes, I understand PCI Bus offsets and yes
I understand that they only apply beyond the bus they're working with.
But this isn't *that* complicated and it should be the responsibility of
the P2PDMA code to sort out and provide a dma_addr_t for. The dma_addr_t
that's passed through the block layer could be a bus address or it could
be the result of a dma_map_* request (if the transaction is found to go
through an RC) depending on the requirements of the devices being used.
> Also duplicating the whole block I/O stack, including hooks all over
> the fast path is pretty much a no-go.
There was very little duplicate code in the patch set. (Really just the
mapping code). There are a few hooks, but in practice not that many if
we ignore the WARN_ONs. We might be able to work to reduce this further.
The main hooks are: when we skip bouncing, when we skip integrity prep,
when we split, and when we map. And the patchset drops the PCI_P2PDMA
hook when we map. So we're talking about maybe three or four extra ifs
that would likely normally be fast due to the branch predictor.
> I've been pondering for a while if we wouldn't be better off just
> passing a phys_addr_t + len instead of the page, offset, len tuple
> in the bio_vec, though. If you look at the normal I/O path here
> is what we normally do:
>
> - we get a page as input, either because we have it at hand (e.g.
> from the page cache) or from get_user_pages (which actually caculates
> it from a pfn in the page tables)
> - once in the bio all the merging decisions are based on the physical
> address, so we have to convert it to the physical address there,
> potentially multiple times
> - then dma mapping all works off the physical address, which it gets
> from the page at the start
> - then only the dma address is used for the I/O
> - on I/O completion we often but not always need the page again. In
> the direct I/O case for reference counting and dirty status, in the
> file system also for things like marking the page uptodate
>
> So if we move to a phys_addr_t we'd need to go back to the page at least
> once. But because of how the merging works we really only need to do
> it once per segment, as we can just do pointer arithmerics do get the
> following pages. As we generally go at least once from a physical
> address to a page in the merging code even a relatively expensive vmem_map
> looks shouldn't be too bad. Even more so given that the super hot path
> (small blkdev direct I/O) can actually trivially cache the affected pages
> as well.
I've always wondered why it wasn't done this way. Passing around a page
pointer *and* an offset always seemed less efficient than just a
physical address. If we did do this, the proposed dma_addr_t and
phys_addr_t paths through the block layer could be a lot more similar as
things like the split calculation could work on either address type.
We'd just have to prevent bouncing and integrity and change have a hook
on how it's mapped.
> Linus kinda hates the pfn approach, but part of that was really that
> it was proposed for file system data, which we all found out really
> can't work as-is without pages the hard way. Another part probably
> was potential performance issue, but between the few page lookups, and
> the fact that using a single phys_addr_t instead of pfn/page + offset
> should avoid quite a few calculations performance should not actually
> be affected, although we'll have to be careful to actually verify that.
Yes, I'd agree that removing the offset should make things simpler. But
that requires changing a lot of stuff and doesn't really help what I'm
trying to do.
Logan
^ permalink raw reply
* [PATCH] IB/hfi1: Avoid hardlockup with flushlist_lock
From: Mike Marciniszyn @ 2019-06-24 15:56 UTC (permalink / raw)
To: stable; +Cc: linux-rdma, stable-commits
commit cf131a81967583ae737df6383a0893b9fee75b4e upstream.
Heavy contention of the sde flushlist_lock can cause hard lockups at
extreme scale when the flushing logic is under stress.
Mitigate by replacing the item at a time copy to the local list with
an O(1) list_splice_init() and using the high priority work queue to
do the flushes.
Ported to linux-4.9.y.
Fixes: 7724105686e7 ("IB/hfi1: add driver files")
Cc: <stable@vger.kernel.org>
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>
---
drivers/infiniband/hw/hfi1/sdma.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/sdma.c b/drivers/infiniband/hw/hfi1/sdma.c
index 9cbe52d..76e63c8 100644
--- a/drivers/infiniband/hw/hfi1/sdma.c
+++ b/drivers/infiniband/hw/hfi1/sdma.c
@@ -410,10 +410,7 @@ static void sdma_flush(struct sdma_engine *sde)
sdma_flush_descq(sde);
spin_lock_irqsave(&sde->flushlist_lock, flags);
/* copy flush list */
- list_for_each_entry_safe(txp, txp_next, &sde->flushlist, list) {
- list_del_init(&txp->list);
- list_add_tail(&txp->list, &flushlist);
- }
+ list_splice_init(&sde->flushlist, &flushlist);
spin_unlock_irqrestore(&sde->flushlist_lock, flags);
/* flush from flush list */
list_for_each_entry_safe(txp, txp_next, &flushlist, list)
@@ -2406,7 +2403,7 @@ int sdma_send_txreq(struct sdma_engine *sde,
wait->tx_count++;
wait->count += tx->num_desc;
}
- schedule_work(&sde->flush_worker);
+ queue_work_on(sde->cpu, system_highpri_wq, &sde->flush_worker);
ret = -ECOMM;
goto unlock;
nodesc:
@@ -2504,7 +2501,7 @@ int sdma_send_txlist(struct sdma_engine *sde, struct iowait *wait,
}
}
spin_unlock(&sde->flushlist_lock);
- schedule_work(&sde->flush_worker);
+ queue_work_on(sde->cpu, system_highpri_wq, &sde->flush_worker);
ret = -ECOMM;
goto update_tail;
nodesc:
^ permalink raw reply related
* Re: [PATCH v18 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
From: Kees Cook @ 2019-06-24 15:04 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander
In-Reply-To: <653598b3cfcd80f0cc69f72a214e156bb1afde68.1561386715.git.andreyknvl@google.com>
On Mon, Jun 24, 2019 at 04:32:47PM +0200, Andrey Konovalov wrote:
> From: Catalin Marinas <catalin.marinas@arm.com>
>
> It is not desirable to relax the ABI to allow tagged user addresses into
> the kernel indiscriminately. This patch introduces a prctl() interface
> for enabling or disabling the tagged ABI with a global sysctl control
> for preventing applications from enabling the relaxed ABI (meant for
> testing user-space prctl() return error checking without reconfiguring
> the kernel). The ABI properties are inherited by threads of the same
> application and fork()'ed children but cleared on execve(). A Kconfig
> option allows the overall disabling of the relaxed ABI.
>
> The PR_SET_TAGGED_ADDR_CTRL will be expanded in the future to handle
> MTE-specific settings like imprecise vs precise exceptions.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
> arch/arm64/Kconfig | 9 ++++
> arch/arm64/include/asm/processor.h | 8 ++++
> arch/arm64/include/asm/thread_info.h | 1 +
> arch/arm64/include/asm/uaccess.h | 4 +-
> arch/arm64/kernel/process.c | 72 ++++++++++++++++++++++++++++
> include/uapi/linux/prctl.h | 5 ++
> kernel/sys.c | 12 +++++
> 7 files changed, 110 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 697ea0510729..55fbaf20af2d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1107,6 +1107,15 @@ config ARM64_SW_TTBR0_PAN
> zeroed area and reserved ASID. The user access routines
> restore the valid TTBR0_EL1 temporarily.
>
> +config ARM64_TAGGED_ADDR_ABI
> + bool "Enable the tagged user addresses syscall ABI"
> + default y
> + help
> + When this option is enabled, user applications can opt in to a
> + relaxed ABI via prctl() allowing tagged addresses to be passed
> + to system calls as pointer arguments. For details, see
> + Documentation/arm64/tagged-address-abi.txt.
> +
> menuconfig COMPAT
> bool "Kernel support for 32-bit EL0"
> depends on ARM64_4K_PAGES || EXPERT
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index fd5b1a4efc70..ee86070a28d4 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -296,6 +296,14 @@ extern void __init minsigstksz_setup(void);
> /* PR_PAC_RESET_KEYS prctl */
> #define PAC_RESET_KEYS(tsk, arg) ptrauth_prctl_reset_keys(tsk, arg)
>
> +#ifdef CONFIG_ARM64_TAGGED_ADDR_ABI
> +/* PR_{SET,GET}_TAGGED_ADDR_CTRL prctl */
> +long set_tagged_addr_ctrl(unsigned long arg);
> +long get_tagged_addr_ctrl(void);
> +#define SET_TAGGED_ADDR_CTRL(arg) set_tagged_addr_ctrl(arg)
> +#define GET_TAGGED_ADDR_CTRL() get_tagged_addr_ctrl()
> +#endif
> +
> /*
> * For CONFIG_GCC_PLUGIN_STACKLEAK
> *
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 2372e97db29c..4f81c4f15404 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -88,6 +88,7 @@ void arch_release_task_struct(struct task_struct *tsk);
> #define TIF_SVE 23 /* Scalable Vector Extension in use */
> #define TIF_SVE_VL_INHERIT 24 /* Inherit sve_vl_onexec across exec */
> #define TIF_SSBD 25 /* Wants SSB mitigation */
> +#define TIF_TAGGED_ADDR 26 /* Allow tagged user addresses */
>
> #define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
> #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index a138e3b4f717..097d6bfac0b7 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -62,7 +62,9 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
> {
> unsigned long ret, limit = current_thread_info()->addr_limit;
>
> - addr = untagged_addr(addr);
> + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) &&
> + test_thread_flag(TIF_TAGGED_ADDR))
> + addr = untagged_addr(addr);
>
> __chk_user_ptr(addr);
> asm volatile(
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 9856395ccdb7..60e70158a4a1 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -19,6 +19,7 @@
> #include <linux/kernel.h>
> #include <linux/mm.h>
> #include <linux/stddef.h>
> +#include <linux/sysctl.h>
> #include <linux/unistd.h>
> #include <linux/user.h>
> #include <linux/delay.h>
> @@ -307,11 +308,18 @@ static void tls_thread_flush(void)
> }
> }
>
> +static void flush_tagged_addr_state(void)
> +{
> + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI))
> + clear_thread_flag(TIF_TAGGED_ADDR);
> +}
> +
> void flush_thread(void)
> {
> fpsimd_flush_thread();
> tls_thread_flush();
> flush_ptrace_hw_breakpoint(current);
> + flush_tagged_addr_state();
> }
>
> void release_thread(struct task_struct *dead_task)
> @@ -541,3 +549,67 @@ void arch_setup_new_exec(void)
>
> ptrauth_thread_init_user(current);
> }
> +
> +#ifdef CONFIG_ARM64_TAGGED_ADDR_ABI
> +/*
> + * Control the relaxed ABI allowing tagged user addresses into the kernel.
> + */
> +static unsigned int tagged_addr_prctl_allowed = 1;
> +
> +long set_tagged_addr_ctrl(unsigned long arg)
> +{
> + if (!tagged_addr_prctl_allowed)
> + return -EINVAL;
> + if (is_compat_task())
> + return -EINVAL;
> + if (arg & ~PR_TAGGED_ADDR_ENABLE)
> + return -EINVAL;
> +
> + update_thread_flag(TIF_TAGGED_ADDR, arg & PR_TAGGED_ADDR_ENABLE);
> +
> + return 0;
> +}
> +
> +long get_tagged_addr_ctrl(void)
> +{
> + if (!tagged_addr_prctl_allowed)
> + return -EINVAL;
> + if (is_compat_task())
> + return -EINVAL;
> +
> + if (test_thread_flag(TIF_TAGGED_ADDR))
> + return PR_TAGGED_ADDR_ENABLE;
> +
> + return 0;
> +}
> +
> +/*
> + * Global sysctl to disable the tagged user addresses support. This control
> + * only prevents the tagged address ABI enabling via prctl() and does not
> + * disable it for tasks that already opted in to the relaxed ABI.
> + */
> +static int zero;
> +static int one = 1;
> +
> +static struct ctl_table tagged_addr_sysctl_table[] = {
> + {
> + .procname = "tagged_addr",
> + .mode = 0644,
> + .data = &tagged_addr_prctl_allowed,
> + .maxlen = sizeof(int),
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &zero,
> + .extra2 = &one,
> + },
> + { }
> +};
> +
> +static int __init tagged_addr_init(void)
> +{
> + if (!register_sysctl("abi", tagged_addr_sysctl_table))
> + return -EINVAL;
> + return 0;
> +}
> +
> +core_initcall(tagged_addr_init);
> +#endif /* CONFIG_ARM64_TAGGED_ADDR_ABI */
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index 094bb03b9cc2..2e927b3e9d6c 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -229,4 +229,9 @@ struct prctl_mm_map {
> # define PR_PAC_APDBKEY (1UL << 3)
> # define PR_PAC_APGAKEY (1UL << 4)
>
> +/* Tagged user address controls for arm64 */
> +#define PR_SET_TAGGED_ADDR_CTRL 55
> +#define PR_GET_TAGGED_ADDR_CTRL 56
> +# define PR_TAGGED_ADDR_ENABLE (1UL << 0)
> +
> #endif /* _LINUX_PRCTL_H */
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 2969304c29fe..c6c4d5358bd3 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -124,6 +124,12 @@
> #ifndef PAC_RESET_KEYS
> # define PAC_RESET_KEYS(a, b) (-EINVAL)
> #endif
> +#ifndef SET_TAGGED_ADDR_CTRL
> +# define SET_TAGGED_ADDR_CTRL(a) (-EINVAL)
> +#endif
> +#ifndef GET_TAGGED_ADDR_CTRL
> +# define GET_TAGGED_ADDR_CTRL() (-EINVAL)
> +#endif
>
> /*
> * this is where the system-wide overflow UID and GID are defined, for
> @@ -2492,6 +2498,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> return -EINVAL;
> error = PAC_RESET_KEYS(me, arg2);
> break;
> + case PR_SET_TAGGED_ADDR_CTRL:
> + error = SET_TAGGED_ADDR_CTRL(arg2);
> + break;
> + case PR_GET_TAGGED_ADDR_CTRL:
> + error = GET_TAGGED_ADDR_CTRL();
> + break;
> default:
> error = -EINVAL;
> break;
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
From: Kees Cook @ 2019-06-24 15:02 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander
In-Reply-To: <61d800c35a4f391218fbca6f05ec458557d8d097.1561386715.git.andreyknvl@google.com>
On Mon, Jun 24, 2019 at 04:32:55PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> In radeon_gem_userptr_ioctl() an MMU notifier is set up with a (tagged)
> userspace pointer. The untagged address should be used so that MMU
> notifiers for the untagged address get correctly matched up with the right
> BO. This funcation also calls radeon_ttm_tt_pin_userptr(), which uses
> provided user pointers for vma lookups, which can only by done with
> untagged pointers.
>
> This patch untags user pointers in radeon_gem_userptr_ioctl().
>
> Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> drivers/gpu/drm/radeon/radeon_gem.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index 44617dec8183..90eb78fb5eb2 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -291,6 +291,8 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, void *data,
> uint32_t handle;
> int r;
>
> + args->addr = untagged_addr(args->addr);
> +
> if (offset_in_page(args->addr | args->size))
> return -EINVAL;
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
From: Kees Cook @ 2019-06-24 15:02 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander
In-Reply-To: <0999c80cd639b78ae27c0674069d552833227564.1561386715.git.andreyknvl@google.com>
On Mon, Jun 24, 2019 at 04:33:00PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> This patch adds a simple test, that calls the uname syscall with a
> tagged user pointer as an argument. Without the kernel accepting tagged
> user pointers the test fails with EFAULT.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> tools/testing/selftests/arm64/.gitignore | 1 +
> tools/testing/selftests/arm64/Makefile | 11 +++++++
> .../testing/selftests/arm64/run_tags_test.sh | 12 ++++++++
> tools/testing/selftests/arm64/tags_test.c | 29 +++++++++++++++++++
> 4 files changed, 53 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/.gitignore
> create mode 100644 tools/testing/selftests/arm64/Makefile
> create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
> create mode 100644 tools/testing/selftests/arm64/tags_test.c
>
> diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore
> new file mode 100644
> index 000000000000..e8fae8d61ed6
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/.gitignore
> @@ -0,0 +1 @@
> +tags_test
> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> new file mode 100644
> index 000000000000..a61b2e743e99
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# ARCH can be overridden by the user for cross compiling
> +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> +
> +ifneq (,$(filter $(ARCH),aarch64 arm64))
> +TEST_GEN_PROGS := tags_test
> +TEST_PROGS := run_tags_test.sh
> +endif
> +
> +include ../lib.mk
> diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh
> new file mode 100755
> index 000000000000..745f11379930
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/run_tags_test.sh
> @@ -0,0 +1,12 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +echo "--------------------"
> +echo "running tags test"
> +echo "--------------------"
> +./tags_test
> +if [ $? -ne 0 ]; then
> + echo "[FAIL]"
> +else
> + echo "[PASS]"
> +fi
> diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c
> new file mode 100644
> index 000000000000..22a1b266e373
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/tags_test.c
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <stdint.h>
> +#include <sys/prctl.h>
> +#include <sys/utsname.h>
> +
> +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56)
> +#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \
> + SHIFT_TAG(tag))
> +
> +int main(void)
> +{
> + static int tbi_enabled = 0;
> + struct utsname *ptr, *tagged_ptr;
> + int err;
> +
> + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
> + tbi_enabled = 1;
> + ptr = (struct utsname *)malloc(sizeof(*ptr));
> + if (tbi_enabled)
> + tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42);
> + err = uname(tagged_ptr);
> + free(ptr);
> +
> + return err;
> +}
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
From: Kees Cook @ 2019-06-24 15:01 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander
In-Reply-To: <ea0ff94ef2b8af12ea6c222c5ebd970e0849b6dd.1561386715.git.andreyknvl@google.com>
On Mon, Jun 24, 2019 at 04:32:56PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
> only by done with untagged pointers.
>
> Untag user pointers in this function.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx4/mr.c b/drivers/infiniband/hw/mlx4/mr.c
> index 355205a28544..13d9f917f249 100644
> --- a/drivers/infiniband/hw/mlx4/mr.c
> +++ b/drivers/infiniband/hw/mlx4/mr.c
> @@ -378,6 +378,7 @@ static struct ib_umem *mlx4_get_umem_mr(struct ib_udata *udata, u64 start,
> * again
> */
> if (!ib_access_writable(access_flags)) {
> + unsigned long untagged_start = untagged_addr(start);
> struct vm_area_struct *vma;
>
> down_read(¤t->mm->mmap_sem);
> @@ -386,9 +387,9 @@ static struct ib_umem *mlx4_get_umem_mr(struct ib_udata *udata, u64 start,
> * cover the memory, but for now it requires a single vma to
> * entirely cover the MR to support RO mappings.
> */
> - vma = find_vma(current->mm, start);
> - if (vma && vma->vm_end >= start + length &&
> - vma->vm_start <= start) {
> + vma = find_vma(current->mm, untagged_start);
> + if (vma && vma->vm_end >= untagged_start + length &&
> + vma->vm_start <= untagged_start) {
> if (vma->vm_flags & VM_WRITE)
> access_flags |= IB_ACCESS_LOCAL_WRITE;
> } else {
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
From: Kees Cook @ 2019-06-24 15:01 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander
In-Reply-To: <61d800c35a4f391218fbca6f05ec458557d8d097.1561386715.git.andreyknvl@google.com>
On Mon, Jun 24, 2019 at 04:32:55PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> In radeon_gem_userptr_ioctl() an MMU notifier is set up with a (tagged)
> userspace pointer. The untagged address should be used so that MMU
> notifiers for the untagged address get correctly matched up with the right
> BO. This funcation also calls radeon_ttm_tt_pin_userptr(), which uses
> provided user pointers for vma lookups, which can only by done with
> untagged pointers.
>
> This patch untags user pointers in radeon_gem_userptr_ioctl().
>
> Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> drivers/gpu/drm/radeon/radeon_gem.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index 44617dec8183..90eb78fb5eb2 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -291,6 +291,8 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, void *data,
> uint32_t handle;
> int r;
>
> + args->addr = untagged_addr(args->addr);
> +
> if (offset_in_page(args->addr | args->size))
> return -EINVAL;
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v18 09/15] drm/amdgpu: untag user pointers
From: Kees Cook @ 2019-06-24 15:00 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab,
Jens Wiklander
In-Reply-To: <1d036fc5bec4be059ee7f4f42bf7417dc44651dd.1561386715.git.andreyknvl@google.com>
On Mon, Jun 24, 2019 at 04:32:54PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends kernel ABI to allow to pass
> tagged user pointers (with the top byte set to something else other than
> 0x00) as syscall arguments.
>
> In amdgpu_gem_userptr_ioctl() and amdgpu_amdkfd_gpuvm.c/init_user_pages()
> an MMU notifier is set up with a (tagged) userspace pointer. The untagged
> address should be used so that MMU notifiers for the untagged address get
> correctly matched up with the right BO. This patch untag user pointers in
> amdgpu_gem_userptr_ioctl() for the GEM case and in amdgpu_amdkfd_gpuvm_
> alloc_memory_of_gpu() for the KFD case. This also makes sure that an
> untagged pointer is passed to amdgpu_ttm_tt_get_user_pages(), which uses
> it for vma lookups.
>
> Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index a6e5184d436c..5d476e9bbc43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1108,7 +1108,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> alloc_flags = 0;
> if (!offset || !*offset)
> return -EINVAL;
> - user_addr = *offset;
> + user_addr = untagged_addr(*offset);
> } else if (flags & ALLOC_MEM_FLAGS_DOORBELL) {
> domain = AMDGPU_GEM_DOMAIN_GTT;
> alloc_domain = AMDGPU_GEM_DOMAIN_CPU;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index d4fcf5475464..e91df1407618 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -287,6 +287,8 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
> uint32_t handle;
> int r;
>
> + args->addr = untagged_addr(args->addr);
> +
> if (offset_in_page(args->addr | args->size))
> return -EINVAL;
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH bpf-next] samples: bpf: Remove bpf_debug macro in favor of bpf_printk
From: Daniel Borkmann @ 2019-06-24 14:59 UTC (permalink / raw)
To: Michal Rostecki
Cc: Doug Ledford, Jason Gunthorpe, Alexei Starovoitov,
Martin KaFai Lau, Song Liu, Yonghong Song, linux-rdma, netdev,
bpf, linux-kernel
In-Reply-To: <20190618181338.24145-1-mrostecki@opensuse.org>
On 06/18/2019 08:13 PM, Michal Rostecki wrote:
> ibumad example was implementing the bpf_debug macro which is exactly the
> same as the bpf_printk macro available in bpf_helpers.h. This change
> makes use of bpf_printk instead of bpf_debug.
>
> Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
Applied, thanks!
^ permalink raw reply
* [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
From: Andrey Konovalov @ 2019-06-24 14:33 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
This patch adds a simple test, that calls the uname syscall with a
tagged user pointer as an argument. Without the kernel accepting tagged
user pointers the test fails with EFAULT.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
tools/testing/selftests/arm64/.gitignore | 1 +
tools/testing/selftests/arm64/Makefile | 11 +++++++
.../testing/selftests/arm64/run_tags_test.sh | 12 ++++++++
tools/testing/selftests/arm64/tags_test.c | 29 +++++++++++++++++++
4 files changed, 53 insertions(+)
create mode 100644 tools/testing/selftests/arm64/.gitignore
create mode 100644 tools/testing/selftests/arm64/Makefile
create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
create mode 100644 tools/testing/selftests/arm64/tags_test.c
diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore
new file mode 100644
index 000000000000..e8fae8d61ed6
--- /dev/null
+++ b/tools/testing/selftests/arm64/.gitignore
@@ -0,0 +1 @@
+tags_test
diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
new file mode 100644
index 000000000000..a61b2e743e99
--- /dev/null
+++ b/tools/testing/selftests/arm64/Makefile
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0
+
+# ARCH can be overridden by the user for cross compiling
+ARCH ?= $(shell uname -m 2>/dev/null || echo not)
+
+ifneq (,$(filter $(ARCH),aarch64 arm64))
+TEST_GEN_PROGS := tags_test
+TEST_PROGS := run_tags_test.sh
+endif
+
+include ../lib.mk
diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh
new file mode 100755
index 000000000000..745f11379930
--- /dev/null
+++ b/tools/testing/selftests/arm64/run_tags_test.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+echo "--------------------"
+echo "running tags test"
+echo "--------------------"
+./tags_test
+if [ $? -ne 0 ]; then
+ echo "[FAIL]"
+else
+ echo "[PASS]"
+fi
diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c
new file mode 100644
index 000000000000..22a1b266e373
--- /dev/null
+++ b/tools/testing/selftests/arm64/tags_test.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <sys/prctl.h>
+#include <sys/utsname.h>
+
+#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56)
+#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \
+ SHIFT_TAG(tag))
+
+int main(void)
+{
+ static int tbi_enabled = 0;
+ struct utsname *ptr, *tagged_ptr;
+ int err;
+
+ if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0)
+ tbi_enabled = 1;
+ ptr = (struct utsname *)malloc(sizeof(*ptr));
+ if (tbi_enabled)
+ tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42);
+ err = uname(tagged_ptr);
+ free(ptr);
+
+ return err;
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v18 14/15] vfio/type1: untag user pointers in vaddr_get_pfn
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
vaddr_get_pfn() uses provided user pointers for vma lookups, which can
only by done with untagged pointers.
Untag user pointers in this function.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/vfio/vfio_iommu_type1.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index add34adfadc7..7b8283e33d10 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -381,6 +381,8 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
down_read(&mm->mmap_sem);
+ vaddr = untagged_addr(vaddr);
+
vma = find_vma_intersection(mm, vaddr, vaddr + 1);
if (vma && vma->vm_flags & VM_PFNMAP) {
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v18 13/15] tee/shm: untag user pointers in tee_shm_register
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
tee_shm_register()->optee_shm_unregister()->check_mem_type() uses provided
user pointers for vma lookups (via __check_mem_type()), which can only by
done with untagged pointers.
Untag user pointers in this function.
Reviewed-by: Kees Cook <keescook@chromium.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/tee/tee_shm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 2da026fd12c9..09ddcd06c715 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -254,6 +254,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr,
shm->teedev = teedev;
shm->ctx = ctx;
shm->id = -1;
+ addr = untagged_addr(addr);
start = rounddown(addr, PAGE_SIZE);
shm->offset = addr - start;
shm->size = length;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v18 12/15] media/v4l2-core: untag user pointers in videobuf_dma_contig_user_get
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
videobuf_dma_contig_user_get() uses provided user pointers for vma
lookups, which can only by done with untagged pointers.
Untag the pointers in this function.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/media/v4l2-core/videobuf-dma-contig.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf-dma-contig.c b/drivers/media/v4l2-core/videobuf-dma-contig.c
index 0491122b03c4..ec554eff29b9 100644
--- a/drivers/media/v4l2-core/videobuf-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf-dma-contig.c
@@ -157,6 +157,7 @@ static void videobuf_dma_contig_user_put(struct videobuf_dma_contig_memory *mem)
static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory *mem,
struct videobuf_buffer *vb)
{
+ unsigned long untagged_baddr = untagged_addr(vb->baddr);
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
unsigned long prev_pfn, this_pfn;
@@ -164,22 +165,22 @@ static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory *mem,
unsigned int offset;
int ret;
- offset = vb->baddr & ~PAGE_MASK;
+ offset = untagged_baddr & ~PAGE_MASK;
mem->size = PAGE_ALIGN(vb->size + offset);
ret = -EINVAL;
down_read(&mm->mmap_sem);
- vma = find_vma(mm, vb->baddr);
+ vma = find_vma(mm, untagged_baddr);
if (!vma)
goto out_up;
- if ((vb->baddr + mem->size) > vma->vm_end)
+ if ((untagged_baddr + mem->size) > vma->vm_end)
goto out_up;
pages_done = 0;
prev_pfn = 0; /* kill warning */
- user_address = vb->baddr;
+ user_address = untagged_baddr;
while (pages_done < (mem->size >> PAGE_SHIFT)) {
ret = follow_pfn(vma, user_address, &this_pfn);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v18 11/15] IB/mlx4: untag user pointers in mlx4_get_umem_mr
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can
only by done with untagged pointers.
Untag user pointers in this function.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mr.c b/drivers/infiniband/hw/mlx4/mr.c
index 355205a28544..13d9f917f249 100644
--- a/drivers/infiniband/hw/mlx4/mr.c
+++ b/drivers/infiniband/hw/mlx4/mr.c
@@ -378,6 +378,7 @@ static struct ib_umem *mlx4_get_umem_mr(struct ib_udata *udata, u64 start,
* again
*/
if (!ib_access_writable(access_flags)) {
+ unsigned long untagged_start = untagged_addr(start);
struct vm_area_struct *vma;
down_read(¤t->mm->mmap_sem);
@@ -386,9 +387,9 @@ static struct ib_umem *mlx4_get_umem_mr(struct ib_udata *udata, u64 start,
* cover the memory, but for now it requires a single vma to
* entirely cover the MR to support RO mappings.
*/
- vma = find_vma(current->mm, start);
- if (vma && vma->vm_end >= start + length &&
- vma->vm_start <= start) {
+ vma = find_vma(current->mm, untagged_start);
+ if (vma && vma->vm_end >= untagged_start + length &&
+ vma->vm_start <= untagged_start) {
if (vma->vm_flags & VM_WRITE)
access_flags |= IB_ACCESS_LOCAL_WRITE;
} else {
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v18 10/15] drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
In radeon_gem_userptr_ioctl() an MMU notifier is set up with a (tagged)
userspace pointer. The untagged address should be used so that MMU
notifiers for the untagged address get correctly matched up with the right
BO. This funcation also calls radeon_ttm_tt_pin_userptr(), which uses
provided user pointers for vma lookups, which can only by done with
untagged pointers.
This patch untags user pointers in radeon_gem_userptr_ioctl().
Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/gpu/drm/radeon/radeon_gem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 44617dec8183..90eb78fb5eb2 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -291,6 +291,8 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, void *data,
uint32_t handle;
int r;
+ args->addr = untagged_addr(args->addr);
+
if (offset_in_page(args->addr | args->size))
return -EINVAL;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v18 09/15] drm/amdgpu: untag user pointers
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
In amdgpu_gem_userptr_ioctl() and amdgpu_amdkfd_gpuvm.c/init_user_pages()
an MMU notifier is set up with a (tagged) userspace pointer. The untagged
address should be used so that MMU notifiers for the untagged address get
correctly matched up with the right BO. This patch untag user pointers in
amdgpu_gem_userptr_ioctl() for the GEM case and in amdgpu_amdkfd_gpuvm_
alloc_memory_of_gpu() for the KFD case. This also makes sure that an
untagged pointer is passed to amdgpu_ttm_tt_get_user_pages(), which uses
it for vma lookups.
Suggested-by: Felix Kuehling <Felix.Kuehling@amd.com>
Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index a6e5184d436c..5d476e9bbc43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1108,7 +1108,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
alloc_flags = 0;
if (!offset || !*offset)
return -EINVAL;
- user_addr = *offset;
+ user_addr = untagged_addr(*offset);
} else if (flags & ALLOC_MEM_FLAGS_DOORBELL) {
domain = AMDGPU_GEM_DOMAIN_GTT;
alloc_domain = AMDGPU_GEM_DOMAIN_CPU;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index d4fcf5475464..e91df1407618 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -287,6 +287,8 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
uint32_t handle;
int r;
+ args->addr = untagged_addr(args->addr);
+
if (offset_in_page(args->addr | args->size))
return -EINVAL;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v18 08/15] userfaultfd: untag user pointers
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
userfaultfd code use provided user pointers for vma lookups, which can
only by done with untagged pointers.
Untag user pointers in validate_range().
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
fs/userfaultfd.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index ae0b8b5f69e6..c2be36a168ca 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1261,21 +1261,23 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,
}
static __always_inline int validate_range(struct mm_struct *mm,
- __u64 start, __u64 len)
+ __u64 *start, __u64 len)
{
__u64 task_size = mm->task_size;
- if (start & ~PAGE_MASK)
+ *start = untagged_addr(*start);
+
+ if (*start & ~PAGE_MASK)
return -EINVAL;
if (len & ~PAGE_MASK)
return -EINVAL;
if (!len)
return -EINVAL;
- if (start < mmap_min_addr)
+ if (*start < mmap_min_addr)
return -EINVAL;
- if (start >= task_size)
+ if (*start >= task_size)
return -EINVAL;
- if (len > task_size - start)
+ if (len > task_size - *start)
return -EINVAL;
return 0;
}
@@ -1325,7 +1327,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
goto out;
}
- ret = validate_range(mm, uffdio_register.range.start,
+ ret = validate_range(mm, &uffdio_register.range.start,
uffdio_register.range.len);
if (ret)
goto out;
@@ -1514,7 +1516,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister)))
goto out;
- ret = validate_range(mm, uffdio_unregister.start,
+ ret = validate_range(mm, &uffdio_unregister.start,
uffdio_unregister.len);
if (ret)
goto out;
@@ -1665,7 +1667,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake)))
goto out;
- ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
+ ret = validate_range(ctx->mm, &uffdio_wake.start, uffdio_wake.len);
if (ret)
goto out;
@@ -1705,7 +1707,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
sizeof(uffdio_copy)-sizeof(__s64)))
goto out;
- ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
+ ret = validate_range(ctx->mm, &uffdio_copy.dst, uffdio_copy.len);
if (ret)
goto out;
/*
@@ -1761,7 +1763,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
sizeof(uffdio_zeropage)-sizeof(__s64)))
goto out;
- ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
+ ret = validate_range(ctx->mm, &uffdio_zeropage.range.start,
uffdio_zeropage.range.len);
if (ret)
goto out;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v18 07/15] fs/namespace: untag user pointers in copy_mount_options
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
In copy_mount_options a user address is being subtracted from TASK_SIZE.
If the address is lower than TASK_SIZE, the size is calculated to not
allow the exact_copy_from_user() call to cross TASK_SIZE boundary.
However if the address is tagged, then the size will be calculated
incorrectly.
Untag the address before subtracting.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
fs/namespace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 7660c2749c96..ec78f7223917 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2994,7 +2994,7 @@ void *copy_mount_options(const void __user * data)
* the remainder of the page.
*/
/* copy_from_user cannot cross TASK_SIZE ! */
- size = TASK_SIZE - (unsigned long)data;
+ size = TASK_SIZE - (unsigned long)untagged_addr(data);
if (size > PAGE_SIZE)
size = PAGE_SIZE;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v18 06/15] mm: untag user pointers in get_vaddr_frames
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
get_vaddr_frames uses provided user pointers for vma lookups, which can
only by done with untagged pointers. Instead of locating and changing
all callers of this function, perform untagging in it.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/frame_vector.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index c64dca6e27c2..c431ca81dad5 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -46,6 +46,8 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
if (WARN_ON_ONCE(nr_frames > vec->nr_allocated))
nr_frames = vec->nr_allocated;
+ start = untagged_addr(start);
+
down_read(&mm->mmap_sem);
locked = 1;
vma = find_vma_intersection(mm, start, start + 1);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v18 05/15] mm: untag user pointers in mm/gup.c
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
mm/gup.c provides a kernel interface that accepts user addresses and
manipulates user pages directly (for example get_user_pages, that is used
by the futex syscall). Since a user can provided tagged addresses, we need
to handle this case.
Add untagging to gup.c functions that use user addresses for vma lookups.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/gup.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/gup.c b/mm/gup.c
index ddde097cf9e4..c37df3d455a2 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -802,6 +802,8 @@ static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
if (!nr_pages)
return 0;
+ start = untagged_addr(start);
+
VM_BUG_ON(!!pages != !!(gup_flags & FOLL_GET));
/*
@@ -964,6 +966,8 @@ int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
struct vm_area_struct *vma;
vm_fault_t ret, major = 0;
+ address = untagged_addr(address);
+
if (unlocked)
fault_flags |= FAULT_FLAG_ALLOW_RETRY;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v18 04/15] mm: untag user pointers passed to memory syscalls
From: Andrey Konovalov @ 2019-06-24 14:32 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher, Christian Koenig,
Mauro Carvalho Chehab, Jens Wiklander, Alex Williamson,
Leon Romanovsky, Luc Van Oostenryck, Dave Martin, Khalid Aziz,
enh <enh@
In-Reply-To: <cover.1561386715.git.andreyknvl@google.com>
This patch is a part of a series that extends kernel ABI to allow to pass
tagged user pointers (with the top byte set to something else other than
0x00) as syscall arguments.
This patch allows tagged pointers to be passed to the following memory
syscalls: get_mempolicy, madvise, mbind, mincore, mlock, mlock2, mprotect,
mremap, msync, munlock, move_pages.
The mmap and mremap syscalls do not currently accept tagged addresses.
Architectures may interpret the tag as a background colour for the
corresponding vma.
Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/madvise.c | 2 ++
mm/mempolicy.c | 3 +++
mm/migrate.c | 2 +-
mm/mincore.c | 2 ++
mm/mlock.c | 4 ++++
mm/mprotect.c | 2 ++
mm/mremap.c | 7 +++++++
mm/msync.c | 2 ++
8 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/mm/madvise.c b/mm/madvise.c
index 628022e674a7..39b82f8a698f 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -810,6 +810,8 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior)
size_t len;
struct blk_plug plug;
+ start = untagged_addr(start);
+
if (!madvise_behavior_valid(behavior))
return error;
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 01600d80ae01..78e0a88b2680 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1360,6 +1360,7 @@ static long kernel_mbind(unsigned long start, unsigned long len,
int err;
unsigned short mode_flags;
+ start = untagged_addr(start);
mode_flags = mode & MPOL_MODE_FLAGS;
mode &= ~MPOL_MODE_FLAGS;
if (mode >= MPOL_MAX)
@@ -1517,6 +1518,8 @@ static int kernel_get_mempolicy(int __user *policy,
int uninitialized_var(pval);
nodemask_t nodes;
+ addr = untagged_addr(addr);
+
if (nmask != NULL && maxnode < nr_node_ids)
return -EINVAL;
diff --git a/mm/migrate.c b/mm/migrate.c
index f2ecc2855a12..d22c45cf36b2 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1616,7 +1616,7 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
goto out_flush;
if (get_user(node, nodes + i))
goto out_flush;
- addr = (unsigned long)p;
+ addr = (unsigned long)untagged_addr(p);
err = -ENODEV;
if (node < 0 || node >= MAX_NUMNODES)
diff --git a/mm/mincore.c b/mm/mincore.c
index c3f058bd0faf..64c322ed845c 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -249,6 +249,8 @@ SYSCALL_DEFINE3(mincore, unsigned long, start, size_t, len,
unsigned long pages;
unsigned char *tmp;
+ start = untagged_addr(start);
+
/* Check the start address: needs to be page-aligned.. */
if (start & ~PAGE_MASK)
return -EINVAL;
diff --git a/mm/mlock.c b/mm/mlock.c
index a90099da4fb4..a72c1eeded77 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -674,6 +674,8 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
unsigned long lock_limit;
int error = -ENOMEM;
+ start = untagged_addr(start);
+
if (!can_do_mlock())
return -EPERM;
@@ -735,6 +737,8 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
{
int ret;
+ start = untagged_addr(start);
+
len = PAGE_ALIGN(len + (offset_in_page(start)));
start &= PAGE_MASK;
diff --git a/mm/mprotect.c b/mm/mprotect.c
index bf38dfbbb4b4..19f981b733bc 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -465,6 +465,8 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
const bool rier = (current->personality & READ_IMPLIES_EXEC) &&
(prot & PROT_READ);
+ start = untagged_addr(start);
+
prot &= ~(PROT_GROWSDOWN|PROT_GROWSUP);
if (grows == (PROT_GROWSDOWN|PROT_GROWSUP)) /* can't be both */
return -EINVAL;
diff --git a/mm/mremap.c b/mm/mremap.c
index fc241d23cd97..64c9a3b8be0a 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -606,6 +606,13 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
LIST_HEAD(uf_unmap_early);
LIST_HEAD(uf_unmap);
+ /*
+ * Architectures may interpret the tag passed to mmap as a background
+ * colour for the corresponding vma. For mremap we don't allow tagged
+ * new_addr to preserve similar behaviour to mmap.
+ */
+ addr = untagged_addr(addr);
+
if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE))
return ret;
diff --git a/mm/msync.c b/mm/msync.c
index ef30a429623a..c3bd3e75f687 100644
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -37,6 +37,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
int unmapped_error = 0;
int error = -EINVAL;
+ start = untagged_addr(start);
+
if (flags & ~(MS_ASYNC | MS_INVALIDATE | MS_SYNC))
goto out;
if (offset_in_page(start))
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox