| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <507F70E8.3030203@redhat.com>
Date: Wed, 17 Oct 2012 22:00:56 -0500
From: Eric Sandeen <sandeen@...hat.com>
To: Ashish Sangwan <ashishsangwan2@...il.com>
CC: tytso@....edu, linux-ext4@...r.kernel.org,
Namjae Jeon <linkinjeon@...il.com>
Subject: Re: [PATCH] e4defrag: Add option -m mtab to e4defrag
On 10/17/12 9:57 PM, Ashish Sangwan wrote:
> On Wed, Oct 17, 2012 at 10:36 PM, Eric Sandeen <sandeen@...hat.com> wrote:
>> On 10/17/12 11:52 AM, Ashish Sangwan wrote:
>>> Currently, e4defrag will not work on machines where /etc/mtab is not present
>>> OR is empty.
>>> This patch does 3 things:
>>> a) Add option "-m mtab" to e4defrag so that user can specify any file to
>>> be scanned for otbtaining mounted filesystems info.
>>
>> Under what conditions would this be useful; i.e. when is mtab information
>> ever in a file other than /etc/mtab?
>
> On embedded systems usually /etc is read-only. In that case /etc/mtab,
> though present, is empty.
> When we tried to run e4defrag on our system, it gave segfault due to
> the reasons mentioned in patch.
Clearly the segfault needs to be fixed, and looking at /proc/mounts makes sense.
But I just wonder about the usefulness of being able to specify an arbitrary
file in lieu of /etc/mtab. Unless there's a clear usecase I'm not sure I'd
add it.
> Then we look into xfs_fsr and found that it has this -m mtab option
> due to which it worked without any problem
> on our system. Also, xfs_fsr scans /proc/mounts first. So we changed
> e4defrag accordingly.
Compatibility with xfs_fsr is the only reason I can think of to do it :)
I just can't imagine why mount information would be anywhere other than
/etc/mtab or /proc/mounts.
Just pushing back on the complexity a little.
Thanks,
-Eric
>> Thanks,
>> -Eric
>>
>>> b) If mtab option is not specified, first we try to scan /proc/mounts, if failed
>>> to access this file, we try with /etc/mtab.
>>> c) In function is_ext4, check if the varibale "mnt_type" is null before
>>> calling strcmp, otherwise segfault would occur if mtab is empty.
>>>
>>> Signed-off-by: Ashish Sangwan <ashish.sangwan2@...il.com>
>>> Signed-off-by: Namjae Jeon <linkinjeon@...il.com>
>>> ---
>>> misc/e4defrag.8.in | 10 ++++++++++
>>> misc/e4defrag.c | 34 ++++++++++++++++++++++------------
>>> 2 files changed, 32 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/misc/e4defrag.8.in b/misc/e4defrag.8.in
>>> index 75e1bc9..7090e51 100644
>>> --- a/misc/e4defrag.8.in
>>> +++ b/misc/e4defrag.8.in
>>> @@ -9,6 +9,9 @@ e4defrag \- online defragmenter for ext4 filesystem
>>> [
>>> .B \-v
>>> ]
>>> +[
>>> +.B \-m mtab
>>> +]
>>> .I target
>>> \&...
>>> .SH DESCRIPTION
>>> @@ -57,6 +60,13 @@ is never defragmented.
>>> .B \-v
>>> Print error messages and the fragmentation count before and after defrag for
>>> each file.
>>> +.TP
>>> +.B \-m mtab
>>> +This option will specify the file to be scanned for obtaining mounted filesystems
>>> +information. If this option is not specified, the default is to use
>>> +.B /proc/mounts .
>>> +If this file is not accessible, e4defrag will try to get required information from
>>> +.B /etc/mtab
>>> .SH NOTES
>>> .B e4defrag
>>> does not support swap file, files in lost+found directory, and files allocated
>>> diff --git a/misc/e4defrag.c b/misc/e4defrag.c
>>> index 4b31d03..3567e9f 100644
>>> --- a/misc/e4defrag.c
>>> +++ b/misc/e4defrag.c
>>> @@ -123,6 +123,7 @@
>>> #define NGMSG_FILE_OPEN "Failed to open"
>>> #define NGMSG_FILE_UNREG "File is not regular file"
>>> #define NGMSG_LOST_FOUND "Can not process \"lost+found\""
>>> +#define _PATH_PROC_MOUNTS "/proc/mounts"
>>>
>>> /* Data type for filesystem-wide blocks number */
>>> typedef unsigned long long ext4_fsblk_t;
>>> @@ -262,10 +263,8 @@ static int fallocate64(int fd, int mode, loff_t offset, loff_t len)
>>> * @dir_path_len: the length of directory.
>>> */
>>> static int get_mount_point(const char *devname, char *mount_point,
>>> - int dir_path_len)
>>> + int dir_path_len, const char *mtab)
>>> {
>>> - /* Refer to /etc/mtab */
>>> - const char *mtab = MOUNTED;
>>> FILE *fp = NULL;
>>> struct mntent *mnt = NULL;
>>> struct stat64 sb;
>>> @@ -278,7 +277,7 @@ static int get_mount_point(const char *devname, char *mount_point,
>>>
>>> fp = setmntent(mtab, "r");
>>> if (fp == NULL) {
>>> - perror("Couldn't access /etc/mtab");
>>> + printf("Couldn't access %s\n", mtab);
>>> return -1;
>>> }
>>>
>>> @@ -313,14 +312,12 @@ static int get_mount_point(const char *devname, char *mount_point,
>>> *
>>> * @file: the file's name.
>>> */
>>> -static int is_ext4(const char *file, char *devname)
>>> +static int is_ext4(const char *file, char *devname, const char *mtab)
>>> {
>>> int maxlen = 0;
>>> int len, ret;
>>> FILE *fp = NULL;
>>> char *mnt_type = NULL;
>>> - /* Refer to /etc/mtab */
>>> - const char *mtab = MOUNTED;
>>> char file_path[PATH_MAX + 1];
>>> struct mntent *mnt = NULL;
>>> struct statfs64 fsbuf;
>>> @@ -345,7 +342,7 @@ static int is_ext4(const char *file, char *devname)
>>>
>>> fp = setmntent(mtab, "r");
>>> if (fp == NULL) {
>>> - perror("Couldn't access /etc/mtab");
>>> + printf("Couldn't access %s", mtab);
>>> return -1;
>>> }
>>>
>>> @@ -374,6 +371,10 @@ static int is_ext4(const char *file, char *devname)
>>> }
>>>
>>> endmntent(fp);
>>> + if (mnt_type == NULL) {
>>> + printf("Could not get mount information from %s\n", mtab);
>>> + return -1;
>>> + }
>>> if (strcmp(mnt_type, FS_EXT4) == 0) {
>>> FREE(mnt_type);
>>> return 0;
>>> @@ -1724,6 +1725,7 @@ int main(int argc, char *argv[])
>>> int success_flag = 0;
>>> char dir_name[PATH_MAX + 1];
>>> char dev_name[PATH_MAX + 1];
>>> + char *mtab = NULL;
>>> struct stat64 buf;
>>> ext2_filsys fs = NULL;
>>>
>>> @@ -1731,7 +1733,7 @@ int main(int argc, char *argv[])
>>> if (argc == 1)
>>> goto out;
>>>
>>> - while ((opt = getopt(argc, argv, "vc")) != EOF) {
>>> + while ((opt = getopt(argc, argv, "vcm:")) != EOF) {
>>> switch (opt) {
>>> case 'v':
>>> mode_flag |= DETAIL;
>>> @@ -1739,11 +1741,19 @@ int main(int argc, char *argv[])
>>> case 'c':
>>> mode_flag |= STATISTIC;
>>> break;
>>> + case 'm':
>>> + mtab = optarg;
>>> + break;
>>> default:
>>> goto out;
>>> }
>>> }
>>> -
>>> + if (!mtab) {
>>> + if (access(_PATH_PROC_MOUNTS, R_OK) == 0)
>>> + mtab = _PATH_PROC_MOUNTS;
>>> + else
>>> + mtab = _PATH_MOUNTED;
>>> + }
>>> if (argc == optind)
>>> goto out;
>>>
>>> @@ -1797,7 +1807,7 @@ int main(int argc, char *argv[])
>>> if (S_ISBLK(buf.st_mode)) {
>>> /* Block device */
>>> strncpy(dev_name, argv[i], strnlen(argv[i], PATH_MAX));
>>> - if (get_mount_point(argv[i], dir_name, PATH_MAX) < 0)
>>> + if (get_mount_point(argv[i], dir_name, PATH_MAX, mtab) < 0)
>>> continue;
>>> if (lstat64(dir_name, &buf) < 0) {
>>> perror(NGMSG_FILE_INFO);
>>> @@ -1833,7 +1843,7 @@ int main(int argc, char *argv[])
>>> * filesystem type checked in get_mount_point()
>>> */
>>> if (arg_type == FILENAME || arg_type == DIRNAME) {
>>> - if (is_ext4(argv[i], dev_name) < 0)
>>> + if (is_ext4(argv[i], dev_name, mtab) < 0)
>>> continue;
>>> if (realpath(argv[i], dir_name) == NULL) {
>>> perror("Couldn't get full path");
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists