Skip to contentSkip to conversation

open08: calling tst_get_bad_addr from LTP API

PATCH New Li Wang 2 participants 6 messages Jul 11, 2018

lore
Share & tools
lore

Li Wang

author
#0 lore raw

PATCH 1/1 open08: calling tst_get_bad_addr from LTP API

From open(2) manual, pathname(unmapped_fname) points outside the accessible address space, then test will result in failure with EFAULT.

LTP tst_get_bad_addr() maps a "guard-page" by using PROT_NONE to provides page not be accessed too. Here we can take use of it to achieve the same purpose.

---

Notes:
Jan,

I'm sorry I didn't think of that function in LTP API when reveiwed your open08 patch.

Here I propose to remove the get_invalid_addr() from open08.c.

Li Wang

 testcases/kernel/syscalls/open/open08.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)
testcases/kernel/syscalls/open/open08.c +2 -12
index 5b24752..c407332 100644
@@ -58,6 +58,7 @@
 #include <signal.h>
 #include <pwd.h>
 #include "tst_test.h"
+#include "tst_get_bad_addr.h"
 
 static char *existing_fname = "open08_testfile";
 static char *toolong_fname = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
@@ -101,17 +102,6 @@ void verify_open(unsigned int i)
 	}
 }
 
-static void *get_invalid_addr(void)
-{
-	char *bad_addr;
-	int len = 2 * 1024 * 1024;
-
-	bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
-		MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
-	SAFE_MUNMAP(bad_addr, len);
-	return bad_addr + len/2;
-}
-
 static void setup(void)
 {
 	int fildes;
@@ -130,7 +120,7 @@ static void setup(void)
 	fildes = SAFE_CREAT(existing_fname, 0600);
 	close(fildes);
 
-	unmapped_fname = get_invalid_addr();
+	unmapped_fname = tst_get_bad_addr(NULL);
 }
 
 static struct tst_test test = {
Signature · 1 line
2.9.5
author
#1 lore raw

PATCH 1/1 open08: calling tst_get_bad_addr from LTP API

Maybe we misled by the code comments before, after thinking over, I guess the "outside your accessible address space" != "outside of process mapped space". So we don't need to do map/unmap to satisfy that.

And, the comments should be more precise if change as:

testcases/kernel/syscalls/open/open08.c +1 -2
@@ -44,8 +44,8 @@
  *        open(2) should fail with EACCES.
  *
  *     6. Attempt to pass an invalid pathname with an address pointing outside
- *        the address space of the process, as the argument to open(), and
- *        expect to get EFAULT.
+ *        the accessible address space of the process, as the

argument to open(),
+ * and expect to get EFAULT.
*/

Quoted 68 lines · Li Wang

From open(2) manual, pathname(unmapped_fname) points outside the accessible address space, then test will result in failure with EFAULT.

LTP tst_get_bad_addr() maps a "guard-page" by using PROT_NONE to provides page not be accessed too. Here we can take use of it to achieve the same purpose.

---

Notes:
Jan,

I'm sorry I didn't think of that function in LTP API when reveiwed your open08 patch.

Here I propose to remove the get_invalid_addr() from open08.c.

Li Wang

 testcases/kernel/syscalls/open/open08.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)
testcases/kernel/syscalls/open/open08.c +2 -13
index 5b24752..c407332 100644
@@ -58,6 +58,7 @@
 #include <signal.h>
 #include <pwd.h>
 #include "tst_test.h"
+#include "tst_get_bad_addr.h"
 
 static char *existing_fname = "open08_testfile";
 static char *toolong_fname = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
@@ -101,17 +102,6 @@ void verify_open(unsigned int i)
        }
 }
 
-static void *get_invalid_addr(void)
-{
-       char *bad_addr;
-       int len = 2 * 1024 * 1024;
-
-       bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
-               MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
-       SAFE_MUNMAP(bad_addr, len);
-       return bad_addr + len/2;
-}
-
 static void setup(void)
 {
        int fildes;
@@ -130,7 +120,7 @@ static void setup(void)
        fildes = SAFE_CREAT(existing_fname, 0600);
        close(fildes);
 
-       unmapped_fname = get_invalid_addr();
+       unmapped_fname = tst_get_bad_addr(NULL);
 }
 
 static struct tst_test test = {
--

2.9.5

--
Mailing list info: https://lists.linux.it/listinfo/ltp

Signature · 2 lines
Regards,
Li Wang
#2 lore raw

PATCH 1/1 open08: calling tst_get_bad_addr from LTP API

----- Original Message -----

Quoted 4 lines

Maybe we misled by the code comments before, after thinking over, I guess the "outside your accessible address space" != "outside of process mapped space". So we don't need to do map/unmap to satisfy that.

That may be the case, I changed it on purpose to match that comment. Though as you pointed out PROT_NONE might be enough too.

Is the motivation for your patch to re-use existing function or have you hit some problem with map/unmap?

Regards,
Jan

author
#3 lore raw

PATCH 1/1 open08: calling tst_get_bad_addr from LTP API

Quoted 8 lines · Jan Stancek

----- Original Message -----

Quoted 4 lines

Maybe we misled by the code comments before, after thinking over, I guess the "outside your accessible address space" != "outside of process mapped space". So we don't need to do map/unmap to satisfy that.

Quoted 4 lines

That may be the case, I changed it on purpose to match that comment. Though as you pointed out PROT_NONE might be enough too.

Is the motivation for your patch to re-use existing function

Yes, firstly my motivation is to re-use the existing function. But then I realized that maybe the original author misunderst
​oo​
d the EFAULT error in open().

So I find open(2) manual and get this:
"EFAULT pathname points outside your *accessible* address space."

​Then I think maybe we need match the manual but not the code comments.

That's all what I thought.​

Quoted 1 line

or have you hit some problem with map/unmap?

No, no other issues.

Quoted 3 lines

Regards,
Jan

Signature · 5 lines
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20180711/a2caab52/attachment.html>
#4 lore raw

PATCH 1/1 open08: calling tst_get_bad_addr from LTP API

----- Original Message -----

Quoted 19 lines · Jan Stancek
Quoted 8 lines

----- Original Message -----

Quoted 4 lines

Maybe we misled by the code comments before, after thinking over, I guess the "outside your accessible address space" != "outside of process mapped space". So we don't need to do map/unmap to satisfy that.

Quoted 4 lines

That may be the case, I changed it on purpose to match that comment. Though as you pointed out PROT_NONE might be enough too.

Is the motivation for your patch to re-use existing function

Yes, firstly my motivation is to re-use the existing function. But then I realized that maybe the original author misunderst
​oo​
d the EFAULT error in open().

Quoted 4 lines

So I find open(2) manual and get this:
"EFAULT pathname points outside your *accessible* address space."

​Then I think maybe we need match the manual but not the code comments.

I'd say we match it already (in stronger form than PROT_NONE).

I don't object to re-using LTP function. Can you send v2 that updates also the comment please?

Regards,
Jan

author
#5 lore raw

PATCH 1/1 open08: calling tst_get_bad_addr from LTP API

Quoted 23 lines · Jan Stancek

----- Original Message -----

Quoted 1 line

On Wed, Jul 11, 2018 at 3:28 PM, Jan Stancek <[email protected]>

Quoted 18 lines · wrote:
Quoted 8 lines

----- Original Message -----

Quoted 4 lines

Maybe we misled by the code comments before, after thinking over, I guess the "outside your accessible address space" != "outside of process mapped space". So we don't need to do map/unmap to satisfy that.

Quoted 4 lines

That may be the case, I changed it on purpose to match that comment. Though as you pointed out PROT_NONE might be enough too.

Is the motivation for your patch to re-use existing function

Yes, firstly my motivation is to re-use the existing function. But then I realized that maybe the original author misunderst
​oo​
d the EFAULT error in open().

Quoted 7 lines
Quoted 4 lines

So I find open(2) manual and get this:
"EFAULT pathname points outside your *accessible* address space."

​Then I think maybe we need match the manual but not the code comments.

I'd say we match it already (in stronger form than PROT_NONE).

Right, map/unmap is more strict.

Quoted 3 lines

I don't object to re-using LTP function. Can you send v2 that updates also the comment please?

​Sure, though it's not much value to change the function, I will do that to make LTP happy.​

Quoted 4 lines

Regards,
Jan

Signature · 5 lines
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20180711/bd2079da/attachment.html>