[LTP] [PATCH] prot_hsymlinks: explicitly close file descriptors

[LTP] [PATCH] prot_hsymlinks: explicitly close file descriptors

[Stanislav Kholmanskikh]

This test case does not need to keep the file descriptors
open after doing creat() on them. So we close them right after
SAFE_CREAT.

Otherwise on NFS it outputs:

TWARN  :  tst_rmdir: rmobj(/tmpdir/ltp-vdIWJBmrkz/provfaUSg) failed: remove(/tmpdir/ltp-vdIWJBmrkz/provfaUSg/tmp_root/hsym) failed; errno=66: Directory not empty

or:
TWARN  :  tst_rmdir: rmobj(/mnt/proXZmq0T) failed: unlink(/mnt/proXZmq0T/root/.nfs00000000000008470000015e) failed; errno=16: Device or resource busy

The same idea as for 8c200cb8e843724afb49fa6617fceec09ac826a5.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 .../security/prot_hsymlinks/prot_hsymlinks.c       |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c b/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
index 558681a..4c05e3f 100644
--- a/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
+++ b/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
@@ -414,7 +414,7 @@ static void ufiles_add(int usr, char *path, int type)
 	struct user_file *ufile = &users[usr].file[file];
 
 	if (type == IS_FILE)
-		SAFE_CREAT(cleanup, path, 0644);
+		close(SAFE_CREAT(cleanup, path, 0644));
 	else
 		SAFE_MKDIR(cleanup, path, 0755);
 
-- 
1.7.1


------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] prot_hsymlinks: explicitly close file descriptors

[chrubis]

Hi!
> This test case does not need to keep the file descriptors
> open after doing creat() on them. So we close them right after
> SAFE_CREAT.
> 
> Otherwise on NFS it outputs:
> 
> TWARN  :  tst_rmdir: rmobj(/tmpdir/ltp-vdIWJBmrkz/provfaUSg) failed: remove(/tmpdir/ltp-vdIWJBmrkz/provfaUSg/tmp_root/hsym) failed; errno=66: Directory not empty
> 
> or:
> TWARN  :  tst_rmdir: rmobj(/mnt/proXZmq0T) failed: unlink(/mnt/proXZmq0T/root/.nfs00000000000008470000015e) failed; errno=16: Device or resource busy
> 
> The same idea as for 8c200cb8e843724afb49fa6617fceec09ac826a5.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  .../security/prot_hsymlinks/prot_hsymlinks.c       |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c b/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
> index 558681a..4c05e3f 100644
> --- a/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
> +++ b/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
> @@ -414,7 +414,7 @@ static void ufiles_add(int usr, char *path, int type)
>  	struct user_file *ufile = &users[usr].file[file];
>  
>  	if (type == IS_FILE)
> -		SAFE_CREAT(cleanup, path, 0644);
> +		close(SAFE_CREAT(cleanup, path, 0644));
>  	else
>  		SAFE_MKDIR(cleanup, path, 0755);
>  

There is quite a lot of places in LTP where we do creat() and then
close(), what about adding SAFE_TOUCH() with touch(1) sematics to the
safe_file_ops.h in LTP lib?

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] prot_hsymlinks: explicitly close file descriptors

[chrubis]

Hi!
> >> This test case does not need to keep the file descriptors
> >> open after doing creat() on them. So we close them right after
> >> SAFE_CREAT.
> >>
> >> Otherwise on NFS it outputs:
> >>
> >> TWARN  :  tst_rmdir: rmobj(/tmpdir/ltp-vdIWJBmrkz/provfaUSg) failed: remove(/tmpdir/ltp-vdIWJBmrkz/provfaUSg/tmp_root/hsym) failed; errno=66: Directory not empty
> >>
> >> or:
> >> TWARN  :  tst_rmdir: rmobj(/mnt/proXZmq0T) failed: unlink(/mnt/proXZmq0T/root/.nfs00000000000008470000015e) failed; errno=16: Device or resource busy
> >>
> >> The same idea as for 8c200cb8e843724afb49fa6617fceec09ac826a5.
> >>
> >> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> >> ---
> >>   .../security/prot_hsymlinks/prot_hsymlinks.c       |    2 +-
> >>   1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c b/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
> >> index 558681a..4c05e3f 100644
> >> --- a/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
> >> +++ b/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
> >> @@ -414,7 +414,7 @@ static void ufiles_add(int usr, char *path, int type)
> >>   	struct user_file *ufile = &users[usr].file[file];
> >>   
> >>   	if (type == IS_FILE)
> >> -		SAFE_CREAT(cleanup, path, 0644);
> >> +		close(SAFE_CREAT(cleanup, path, 0644));
> >>   	else
> >>   		SAFE_MKDIR(cleanup, path, 0755);
> >>   
> > There is quite a lot of places in LTP where we do creat() and then
> > close(), what about adding SAFE_TOUCH() with touch(1) sematics to the
> > safe_file_ops.h in LTP lib?
> I'm ok with that, but I have two little questions:
> 
> 1. About the semantics.
> We use creat() with a 'mode' parameter. Should it just be passed to 
> open() (with O_CREAT) or should we explicitly call chmod(mode) after 
> open()/close()?
> In other words, should SAFE_TOUCH try to change the permissions if the 
> file exists?

I would go for changing the permissions if the file exists.

> At the moment I think about the following prototype:
> void safe_touch(const char *file, const int lineno,
>                            void (*cleanup_fn)(void),
>                            const char *pathname,
>                            mode_t mode, const struct timespec *times)

The timespec is used for specifying creation time?

> 2. Maybe move SAFE_TOUCH to safe_macros.h ?

I was thinking of safe_file_ops because safe_macros contains wrappers
around various libcalls and touch is not really libcall of any kind. But
that is minor.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
November Webinars for C, C++, Fortran Developers
Accelerate application performance with scalable programming models. Explore
techniques for threading, error checking, porting, and tuning. Get the most 
from the latest Intel processors and coprocessors. See abstracts and register
http://pubads.g.doubleclick.net/gampad/clk?id=60136231&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V2 1/2] Implemented SAFE_TOUCH macro

[Stanislav Kholmanskikh]

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 include/safe_file_ops.h |   21 +++++++++++++++++++++
 lib/safe_file_ops.c     |   41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/include/safe_file_ops.h b/include/safe_file_ops.h
index 7ff5baa..77ad594 100644
--- a/include/safe_file_ops.h
+++ b/include/safe_file_ops.h
@@ -72,4 +72,25 @@ void safe_cp(const char *file, const int lineno,
 #define SAFE_CP(cleanup_fn, src, dst) \
 	safe_cp(__FILE__, __LINE__, (cleanup_fn), (src), (dst))
 
+/*
+ * Safe function to touch a file.
+ *
+ * If the file (pathname) does not exist It will be created with
+ * the specified permission (mode) and the access/modification times (times).
+ *
+ * If mode is 0 then the file is created with (0666 & ~umask)
+ * permission or (if the file exists) the permission is not changed.
+ *
+ * times is a timespec[2] (as for utimensat(2)). If times is NULL then
+ * the access/modification times of the file is set to the current time.
+ */
+void safe_touch(const char *file, const int lineno,
+		void (*cleanup_fn)(void),
+		const char *pathname,
+		mode_t mode, const struct timespec times[2]);
+
+#define SAFE_TOUCH(cleanup_fn, pathname, mode, times) \
+	safe_touch(__FILE__, __LINE__, (cleanup_fn), \
+			(pathname), (mode), (times))
+
 #endif /* SAFE_FILE_OPS */
diff --git a/lib/safe_file_ops.c b/lib/safe_file_ops.c
index b602345..8cfd264 100644
--- a/lib/safe_file_ops.c
+++ b/lib/safe_file_ops.c
@@ -23,6 +23,9 @@
 
 #include <stdarg.h>
 #include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 
 #include "safe_file_ops.h"
 
@@ -152,3 +155,41 @@ void safe_cp(const char *file, const int lineno,
 			 src, dst, file, lineno);
 	}
 }
+
+void safe_touch(const char *file, const int lineno,
+		void (*cleanup_fn)(void),
+		const char *pathname,
+		mode_t mode, const struct timespec times[2])
+{
+	int ret;
+	mode_t defmode;
+
+	defmode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
+
+	ret = open(pathname, O_CREAT | O_WRONLY, defmode);
+	if (ret == -1)
+		tst_brkm(TBROK | TERRNO, cleanup_fn,
+			"Failed to open file '%s' at %s:%d",
+			pathname, file, lineno);
+
+	ret = close(ret);
+	if (ret == -1)
+		tst_brkm(TBROK | TERRNO, cleanup_fn,
+			"Failed to close file '%s' at %s:%d",
+			pathname, file, lineno);
+
+	if (mode != 0) {
+		ret = chmod(pathname, mode);
+		if (ret == -1)
+			tst_brkm(TBROK | TERRNO, cleanup_fn,
+				"Failed to chmod file '%s' at %s:%d",
+				pathname, file, lineno);
+	}
+
+	ret = utimensat(AT_FDCWD, pathname, times, 0);
+	if (ret == -1)
+		tst_brkm(TBROK | TERRNO, cleanup_fn,
+			"Failed to do utimensat() on file '%s' at %s:%d",
+			pathname, file, lineno);
+}
+
-- 
1.7.1


------------------------------------------------------------------------------
November Webinars for C, C++, Fortran Developers
Accelerate application performance with scalable programming models. Explore
techniques for threading, error checking, porting, and tuning. Get the most 
from the latest Intel processors and coprocessors. See abstracts and register
http://pubads.g.doubleclick.net/gampad/clk?id=60136231&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V2 2/2] prot_hsymlinks: use SAFE_TOUCH

[Stanislav Kholmanskikh]

This test case does not need to keep the file descriptors
open after doing creat() on them. So we use SAFE_TOUCH instead
of SAFE_CREAT.

Otherwise on NFS it outputs:

TWARN  :  tst_rmdir: rmobj(/tmpdir/ltp-vdIWJBmrkz/provfaUSg) failed: remove(/tmpdir/ltp-vdIWJBmrkz/provfaUSg/tmp_root/hsym) failed; errno=66: Directory not empty

or:
TWARN  :  tst_rmdir: rmobj(/mnt/proXZmq0T) failed: unlink(/mnt/proXZmq0T/root/.nfs00000000000008470000015e) failed; errno=16: Device or resource busy

The same idea as for 8c200cb8e843724afb49fa6617fceec09ac826a5.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 .../security/prot_hsymlinks/prot_hsymlinks.c       |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c b/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
index 558681a..e9948e3 100644
--- a/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
+++ b/testcases/kernel/security/prot_hsymlinks/prot_hsymlinks.c
@@ -45,6 +45,7 @@
 #include "test.h"
 #include "usctest.h"
 #include "safe_macros.h"
+#include "safe_file_ops.h"
 
 char *TCID = "prot_hsymlinks";
 int TST_TOTAL = 396;
@@ -414,7 +415,7 @@ static void ufiles_add(int usr, char *path, int type)
 	struct user_file *ufile = &users[usr].file[file];
 
 	if (type == IS_FILE)
-		SAFE_CREAT(cleanup, path, 0644);
+		SAFE_TOUCH(cleanup, path, 0644, NULL);
 	else
 		SAFE_MKDIR(cleanup, path, 0755);
 
-- 
1.7.1


------------------------------------------------------------------------------
November Webinars for C, C++, Fortran Developers
Accelerate application performance with scalable programming models. Explore
techniques for threading, error checking, porting, and tuning. Get the most 
from the latest Intel processors and coprocessors. See abstracts and register
http://pubads.g.doubleclick.net/gampad/clk?id=60136231&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V2 1/2] Implemented SAFE_TOUCH macro

[chrubis]

Hi!
> +	ret = open(pathname, O_CREAT | O_WRONLY, defmode);
> +	if (ret == -1)
> +		tst_brkm(TBROK | TERRNO, cleanup_fn,
> +			"Failed to open file '%s' at %s:%d",
> +			pathname, file, lineno);

I (and LKML coding style) prefer to have curly braces around multiline
blocks, but that is very minor...

Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
November Webinars for C, C++, Fortran Developers
Accelerate application performance with scalable programming models. Explore
techniques for threading, error checking, porting, and tuning. Get the most 
from the latest Intel processors and coprocessors. See abstracts and register
http://pubads.g.doubleclick.net/gampad/clk?id=60136231&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list