[PATCH for-rdma] IB/mlx5: Allow mapping the free running counter on PROT_EXEC

[PATCH for-rdma] IB/mlx5: Allow mapping the free running counter on PROT_EXEC

[Matan Barak]

The current mlx5 code disallows mapping the free running counter of
mlx5 based hardwares when PROT_EXEC is set.
Although this behaviour is correct, Linux does add an implicit VM_EXEC
to the vm_flags if the READ_IMPLIES_EXEC bit is set in the process
personality. This happens for example if the process stack is
executable.

This causes libmlx5 to output a warning and prevents the user from
reading the free running clock.
Executing the init segment of the hardware isn't a security risk
(at least no more than executing a process own stack), so we just
prevent writes to there.

Fixes: d69e3bcf7976 ('IB/mlx5: Mmap the HCA's core clock register to
		      user-space')
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---

Hi Doug,

This patch fixes a small issue that occurs when using libmlx5 with
READ_IMPLIES_EXEC. When libmlx5 initializes, it mmaps the free running
counter clock with PROT_READ permissions. Using READ_IMPLIES_EXEC,
PROT_EXEC permission is automatically added and causes mmap to fail.
We allow PROT_EXEC mapping, as we don't see it imposes any security
risk.

Regards,
Matan

 drivers/infiniband/hw/mlx5/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 5acf346..d7b114b 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -1108,7 +1108,7 @@ static int mlx5_ib_mmap(struct ib_ucontext *ibcontext, struct vm_area_struct *vm
 		if (vma->vm_end - vma->vm_start != PAGE_SIZE)
 			return -EINVAL;
 
-		if (vma->vm_flags & (VM_WRITE | VM_EXEC))
+		if (vma->vm_flags & VM_WRITE)
 			return -EPERM;
 
 		/* Don't expose to user-space information it shouldn't have */
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-rdma] IB/mlx5: Allow mapping the free running counter on PROT_EXEC

[Christoph Hellwig]

And why the hack do we set PROT_EXEC on a mapping of device resources?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-rdma] IB/mlx5: Allow mapping the free running counter on PROT_EXEC

[Matan Barak (External)]

On 14/04/2016 16:56, Christoph Hellwig wrote:
> And why the hack do we set PROT_EXEC on a mapping of device resources?
>

Of course we don't (there's no good reason to do that).
However, as written in the commit message, when READ_IMPLIES_EXEC is set 
in current->personality (mm/mmap.c):

if ((prot & PROT_READ) && (current->personality & READ_IMPLIES_EXEC))
	if (!(file && path_noexec(&file->f_path)))
		prot |= PROT_EXEC;

So, we don't want to fail in these cases.

Regards,
Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-rdma] IB/mlx5: Allow mapping the free running counter on PROT_EXEC

[Christoph Hellwig]

On Thu, Apr 14, 2016 at 05:05:42PM +0300, Matan Barak (External) wrote:
> On 14/04/2016 16:56, Christoph Hellwig wrote:
> >And why the hack do we set PROT_EXEC on a mapping of device resources?
> >
> 
> Of course we don't (there's no good reason to do that).
> However, as written in the commit message, when READ_IMPLIES_EXEC is set in
> current->personality (mm/mmap.c):
> 
> if ((prot & PROT_READ) && (current->personality & READ_IMPLIES_EXEC))
> 	if (!(file && path_noexec(&file->f_path)))
> 		prot |= PROT_EXEC;
> 
> So, we don't want to fail in these cases.

Uh, ok - that's nasty.  I somehow only remembered and exec implies read
in x86.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-rdma] IB/mlx5: Allow mapping the free running counter on PROT_EXEC

[Haggai Eran]

On 4/14/2016 4:56 PM, Christoph Hellwig wrote:
> And why the hack do we set PROT_EXEC on a mapping of device resources?

It is set automatically in the kernel for legacy processes that have the
READ_IMPLIES_EXEC personality.

See:
http://lxr.free-electrons.com/source/mm/mmap.c?v=4.5#L1285
https://lwn.net/Articles/94068/
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-rdma] IB/mlx5: Allow mapping the free running counter on PROT_EXEC

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]

On 04/14/2016 09:52 AM, Matan Barak wrote:
> The current mlx5 code disallows mapping the free running counter of
> mlx5 based hardwares when PROT_EXEC is set.
> Although this behaviour is correct, Linux does add an implicit VM_EXEC
> to the vm_flags if the READ_IMPLIES_EXEC bit is set in the process
> personality. This happens for example if the process stack is
> executable.
> 
> This causes libmlx5 to output a warning and prevents the user from
> reading the free running clock.
> Executing the init segment of the hardware isn't a security risk
> (at least no more than executing a process own stack), so we just
> prevent writes to there.
> 
> Fixes: d69e3bcf7976 ('IB/mlx5: Mmap the HCA's core clock register to
> 		      user-space')
> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Thanks, applied.


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]