diff mbox series

[v5,1/3] Add SAFE_MPROTECT() macro

Message ID 20240313092331.18069-2-andrea.cervesato@suse.de
State Accepted
Headers show
Series SysV IPC bug reproducer | expand

Commit Message

Andrea Cervesato March 13, 2024, 9:23 a.m. UTC 
From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
Moved SAFE_MPROTECT() macro into tst_safe_macros.h

 include/tst_safe_macros.h | 60 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

Comments

Cyril Hrubis March 13, 2024, 9:56 a.m. UTC | #1 
Hi!
> +static void prot_to_str(const int prot, char *buf)
> +{
> +	char *ptr = buf;
>

I would still put an explicit check for the buffer size here, so that we
are sure that the complete combination of READ|WRITE|EXEC would fit, but
I guess that it's fine as long as this is an internal static function
and not part of the API.

> +	if (prot == PROT_NONE) {
> +		strcpy(buf, "PROT_NONE");
> +		return;
> +	}
> +
> +	if (prot & PROT_READ) {
> +		strcpy(ptr, PROT_FLAG_STR(PROT_READ));
> +		ptr += sizeof(PROT_FLAG_STR(PROT_READ)) - 1;
> +	}
> +
> +	if (prot & PROT_WRITE) {
> +		strcpy(ptr, PROT_FLAG_STR(PROT_WRITE));
> +		ptr += sizeof(PROT_FLAG_STR(PROT_WRITE)) - 1;
> +	}
> +
> +	if (prot & PROT_EXEC) {
> +		strcpy(ptr, PROT_FLAG_STR(PROT_EXEC));
> +		ptr += sizeof(PROT_FLAG_STR(PROT_EXEC)) - 1;
> +	}
> +
> +	if (buf != ptr)
> +		ptr[-3] = 0;
> +}
> +
>  static inline void *safe_mmap(const char *file, const int lineno,
>                                void *addr, size_t length,
>                                int prot, int flags, int fd, off_t offset)
> @@ -287,6 +318,35 @@ static inline void *safe_mmap(const char *file, const int lineno,
>  	safe_mmap(__FILE__, __LINE__, (addr), (length), (prot), \
>  	(flags), (fd), (offset))
>  
> +static inline int safe_mprotect(const char *file, const int lineno,
> +	char *addr, size_t len, int prot)
> +{
> +	int rval;
> +	char *prot_buf;
> +
> +	prot_buf = (char*) safe_malloc(file, lineno, 0, 512);

Why are we allocating the buffer? Why not just prot_buf[512] ?

Also the cast to (char*) is never needed in C as void* is automatically
converted to any type of a pointer without explicit cast.


Otherwise it looks good. You can add my Reviewed-by: if you change the
malloc to an array on the stack.
Cyril Hrubis March 13, 2024, 9:59 a.m. UTC | #2 
Hi!
> +static inline int safe_mprotect(const char *file, const int lineno,
> +	char *addr, size_t len, int prot)
> +{
> +	int rval;
> +	char *prot_buf;
> +
> +	prot_buf = (char*) safe_malloc(file, lineno, 0, 512);
> +	prot_to_str(prot, prot_buf);
> +
> +	tst_res_(file, lineno, TDEBUG,
> +		"mprotect(%p, %ld, %s(%x))", addr, len, prot_buf, prot);
> +
> +	free(prot_buf);
> +
> +	rval = mprotect(addr, len, prot);
> +
> +	if (rval == -1) {
> +		tst_brk_(file, lineno, TBROK | TERRNO,
> +			"mprotect() failed");
> +	} else if (rval) {
> +		tst_brk_(file, lineno, TBROK | TERRNO,
> +			"Invalid mprotect() return value %d", rval);
> +	}


Ah, and can we please print the whole parameter list for mprotect() in
these two cases as well?

> +	return rval;
> +}
> +#define SAFE_MPROTECT(addr, len, prot) \
> +	safe_mprotect(__FILE__, __LINE__, (addr), (len), (prot))
> +
>  static inline int safe_ftruncate(const char *file, const int lineno,
>                                   int fd, off_t length)
>  {
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp
diff mbox series

Patch

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index 2d497f344..15f914619 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -6,6 +6,7 @@ 
 #ifndef TST_SAFE_MACROS_H__
 #define TST_SAFE_MACROS_H__
 
+#include <stdlib.h>
 #include <sys/mman.h>
 #include <sys/types.h>
 #include <sys/time.h>
@@ -268,6 +269,36 @@  int safe_getgroups(const char *file, const int lineno, int size, gid_t list[]);
  * -D_FILE_OFFSET_BITS=64 compile flag
  */
 
+#define PROT_FLAG_STR(f) #f " | "
+
+static void prot_to_str(const int prot, char *buf)
+{
+	char *ptr = buf;
+
+	if (prot == PROT_NONE) {
+		strcpy(buf, "PROT_NONE");
+		return;
+	}
+
+	if (prot & PROT_READ) {
+		strcpy(ptr, PROT_FLAG_STR(PROT_READ));
+		ptr += sizeof(PROT_FLAG_STR(PROT_READ)) - 1;
+	}
+
+	if (prot & PROT_WRITE) {
+		strcpy(ptr, PROT_FLAG_STR(PROT_WRITE));
+		ptr += sizeof(PROT_FLAG_STR(PROT_WRITE)) - 1;
+	}
+
+	if (prot & PROT_EXEC) {
+		strcpy(ptr, PROT_FLAG_STR(PROT_EXEC));
+		ptr += sizeof(PROT_FLAG_STR(PROT_EXEC)) - 1;
+	}
+
+	if (buf != ptr)
+		ptr[-3] = 0;
+}
+
 static inline void *safe_mmap(const char *file, const int lineno,
                               void *addr, size_t length,
                               int prot, int flags, int fd, off_t offset)
@@ -287,6 +318,35 @@  static inline void *safe_mmap(const char *file, const int lineno,
 	safe_mmap(__FILE__, __LINE__, (addr), (length), (prot), \
 	(flags), (fd), (offset))
 
+static inline int safe_mprotect(const char *file, const int lineno,
+	char *addr, size_t len, int prot)
+{
+	int rval;
+	char *prot_buf;
+
+	prot_buf = (char*) safe_malloc(file, lineno, 0, 512);
+	prot_to_str(prot, prot_buf);
+
+	tst_res_(file, lineno, TDEBUG,
+		"mprotect(%p, %ld, %s(%x))", addr, len, prot_buf, prot);
+
+	free(prot_buf);
+
+	rval = mprotect(addr, len, prot);
+
+	if (rval == -1) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"mprotect() failed");
+	} else if (rval) {
+		tst_brk_(file, lineno, TBROK | TERRNO,
+			"Invalid mprotect() return value %d", rval);
+	}
+
+	return rval;
+}
+#define SAFE_MPROTECT(addr, len, prot) \
+	safe_mprotect(__FILE__, __LINE__, (addr), (len), (prot))
+
 static inline int safe_ftruncate(const char *file, const int lineno,
                                  int fd, off_t length)
 {