sysvshm: Don't mprotect segments with PROT_EXEC

Submitted by Pavel Emelianov on July 16, 2016, 9:24 a.m.

Details

Message ID 5789FD4F.4000302@virtuozzo.com
State Accepted
Series "sysvshm: Don't mprotect segments with PROT_EXEC"
Commit 64303d7fc25cf7d0ccfa71f2bafc5daa1243881d
Headers show

Commit Message

Pavel Emelianov July 16, 2016, 9:24 a.m.
When fixing mprotected (ro) sysvshmems I used the PROT_EXEC flag
to keep the information about whether the segment itself should
be rw or ro. This flag leaked to sys_mprotect and some attachments
of the segment became executable after restore.

Fix this by dropping the EXEC flag.

https://github.com/xemul/criu/issues/180

Reported-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>

---

Patch hide | download patch | download mbox

diff --git a/criu/pie/restorer.c b/criu/pie/restorer.c
index 424ccf3..366ccf0 100644
--- a/criu/pie/restorer.c
+++ b/criu/pie/restorer.c
@@ -517,13 +517,14 @@  static long restore_self_exe_late(struct task_restore_args *args)
 	return ret;
 }
 
-static unsigned long restore_mapping(const VmaEntry *vma_entry)
+static unsigned long restore_mapping(VmaEntry *vma_entry)
 {
 	int prot	= vma_entry->prot;
 	int flags	= vma_entry->flags | MAP_FIXED;
 	unsigned long addr;
 
 	if (vma_entry_is(vma_entry, VMA_AREA_SYSVIPC)) {
+		int att_flags;
 		/*
 		 * See comment in open_shmem_sysv() for what SYSV_SHMEM_SKIP_FD
 		 * means and why we check for PROT_EXEC few lines below.
@@ -531,9 +532,14 @@  static unsigned long restore_mapping(const VmaEntry *vma_entry)
 		if (vma_entry->fd == SYSV_SHMEM_SKIP_FD)
 			return vma_entry->start;
 
+		if (vma_entry->prot & PROT_EXEC) {
+			att_flags = 0;
+			vma_entry->prot &= ~PROT_EXEC;
+		} else
+			att_flags = SHM_RDONLY;
+
 		pr_info("Attach SYSV shmem %d at %"PRIx64"\n", (int)vma_entry->fd, vma_entry->start);
-		return sys_shmat(vma_entry->fd, decode_pointer(vma_entry->start),
-				vma_entry->prot & PROT_EXEC ? 0 : SHM_RDONLY);
+		return sys_shmat(vma_entry->fd, decode_pointer(vma_entry->start), att_flags);
 	}
 
 	/*

Comments

Andrey Vagin July 18, 2016, 9:59 p.m.
On Sat, Jul 16, 2016 at 12:24:31PM +0300, Pavel Emelyanov wrote:
> When fixing mprotected (ro) sysvshmems I used the PROT_EXEC flag
> to keep the information about whether the segment itself should
> be rw or ro. This flag leaked to sys_mprotect and some attachments
> of the segment became executable after restore.
> 
> Fix this by dropping the EXEC flag.
> 
> https://github.com/xemul/criu/issues/180
>

Acked-by: Andrew Vagin <avagin@virtuozzo.com>
 
> Reported-by: Andrey Vagin <avagin@openvz.org>
> Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>
> 
> ---
> 
> diff --git a/criu/pie/restorer.c b/criu/pie/restorer.c
> index 424ccf3..366ccf0 100644
> --- a/criu/pie/restorer.c
> +++ b/criu/pie/restorer.c
> @@ -517,13 +517,14 @@ static long restore_self_exe_late(struct task_restore_args *args)
>  	return ret;
>  }
>  
> -static unsigned long restore_mapping(const VmaEntry *vma_entry)
> +static unsigned long restore_mapping(VmaEntry *vma_entry)
>  {
>  	int prot	= vma_entry->prot;
>  	int flags	= vma_entry->flags | MAP_FIXED;
>  	unsigned long addr;
>  
>  	if (vma_entry_is(vma_entry, VMA_AREA_SYSVIPC)) {
> +		int att_flags;
>  		/*
>  		 * See comment in open_shmem_sysv() for what SYSV_SHMEM_SKIP_FD
>  		 * means and why we check for PROT_EXEC few lines below.
> @@ -531,9 +532,14 @@ static unsigned long restore_mapping(const VmaEntry *vma_entry)
>  		if (vma_entry->fd == SYSV_SHMEM_SKIP_FD)
>  			return vma_entry->start;
>  
> +		if (vma_entry->prot & PROT_EXEC) {
> +			att_flags = 0;
> +			vma_entry->prot &= ~PROT_EXEC;
> +		} else
> +			att_flags = SHM_RDONLY;
> +
>  		pr_info("Attach SYSV shmem %d at %"PRIx64"\n", (int)vma_entry->fd, vma_entry->start);
> -		return sys_shmat(vma_entry->fd, decode_pointer(vma_entry->start),
> -				vma_entry->prot & PROT_EXEC ? 0 : SHM_RDONLY);
> +		return sys_shmat(vma_entry->fd, decode_pointer(vma_entry->start), att_flags);
>  	}
>  
>  	/*
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu