From: Chen Gang <gang.chen@asianux.com>
To: Theodore Ts'o <tytso@mit.edu>, adilger.kernel@dilger.ca
Cc: akpm@linux-foundation.org, jack@suse.cz, linux-ext4@vger.kernel.org
Subject: Re: [Suggestion] fs/ext3: memory leak by calling set_qf_name or clear_qf_name, many times.
Date: Thu, 27 Dec 2012 16:06:02 +0800 [thread overview]
Message-ID: <50DC016A.9070801@asianux.com> (raw)
In-Reply-To: <50DA857B.1050808@asianux.com>
Really (just as Theodore Ts'o said), for ext4 is the same thing as ext3 !
so please help check it by ext4 maintainers, or ext3 maintainers
if want to let me to send relative patch, please tell me, I will try.
:-)
thanks.
gchen
于 2012年12月26日 13:04, Chen Gang 写道:
> Hello Theodore Ts'o
>
> in fs/ext3/supper.c
> for function set_qf_name:
> sbi->s_qf_names[qtype] may already have owned a memory (line 919..925)
> we set sbi->s_qf_names[qtype] = qname directly without checking (line 926)
>
> for function clear_qf_name:
> we set sbi->s_qf_names[qtype] = NULL (line 942..952)
>
>
> for function parse_options:
> we can call set_qf_name or clear_qf_name with USR or GRP many times.
> we find parameters not mind whether they are repeated. (line 975..985)
> so we may call set_qf_name or clear_qf_name several times.
> also may first call set_qf_name, then call clear_qf_name.
>
> in this situation, we will get memory leak.
>
> please help check this suggestion whether valid (I find it by code review).
>
>
> regards.
>
> gchen.
>
> 901 static int set_qf_name(struct super_block *sb, int qtype, substring_t *args)
> 902 {
> 903 struct ext3_sb_info *sbi = EXT3_SB(sb);
> 904 char *qname;
> 905
> 906 if (sb_any_quota_loaded(sb) &&
> 907 !sbi->s_qf_names[qtype]) {
> 908 ext3_msg(sb, KERN_ERR,
> 909 "Cannot change journaled "
> 910 "quota options when quota turned on");
> 911 return 0;
> 912 }
> 913 qname = match_strdup(args);
> 914 if (!qname) {
> 915 ext3_msg(sb, KERN_ERR,
> 916 "Not enough memory for storing quotafile name");
> 917 return 0;
> 918 }
> 919 if (sbi->s_qf_names[qtype] &&
> 920 strcmp(sbi->s_qf_names[qtype], qname)) {
> 921 ext3_msg(sb, KERN_ERR,
> 922 "%s quota file already specified", QTYPE2NAME(qtype));
> 923 kfree(qname);
> 924 return 0;
> 925 }
> 926 sbi->s_qf_names[qtype] = qname;
> 927 if (strchr(sbi->s_qf_names[qtype], '/')) {
> 928 ext3_msg(sb, KERN_ERR,
> 929 "quotafile must be on filesystem root");
> 930 kfree(sbi->s_qf_names[qtype]);
> 931 sbi->s_qf_names[qtype] = NULL;
> 932 return 0;
> 933 }
> 934 set_opt(sbi->s_mount_opt, QUOTA);
> 935 return 1;
> 936 }
> 937
> 938 static int clear_qf_name(struct super_block *sb, int qtype) {
> 939
> 940 struct ext3_sb_info *sbi = EXT3_SB(sb);
> 941
> 942 if (sb_any_quota_loaded(sb) &&
> 943 sbi->s_qf_names[qtype]) {
> 944 ext3_msg(sb, KERN_ERR, "Cannot change journaled quota options"
> 945 " when quota turned on");
> 946 return 0;
> 947 }
> 948 /*
> 949 * The space will be released later when all options are confirmed
> 950 * to be correct
> 951 */
> 952 sbi->s_qf_names[qtype] = NULL;
> 953 return 1;
> 954 }
> 955 #endif
> 956
> 957 static int parse_options (char *options, struct super_block *sb,
> 958 unsigned int *inum, unsigned long *journal_devnum,
> 959 ext3_fsblk_t *n_blocks_count, int is_remount)
> 960 {
> 961 struct ext3_sb_info *sbi = EXT3_SB(sb);
> 962 char * p;
> 963 substring_t args[MAX_OPT_ARGS];
> 964 int data_opt = 0;
> 965 int option;
> 966 kuid_t uid;
> 967 kgid_t gid;
> 968 #ifdef CONFIG_QUOTA
> 969 int qfmt;
> 970 #endif
> 971
> 972 if (!options)
> 973 return 1;
> 974
> 975 while ((p = strsep (&options, ",")) != NULL) {
> 976 int token;
> 977 if (!*p)
> 978 continue;
> 979 /*
> 980 * Initialize args struct so we know whether arg was
> 981 * found; some options take optional arguments.
> 982 */
> 983 args[0].to = args[0].from = NULL;
> 984 token = match_token(p, tokens, args);
> 985 switch (token) {
> 986 case Opt_bsd_df:
> 987 clear_opt (sbi->s_mount_opt, MINIX_DF);
> 988 break;
> 989 case Opt_minix_df:
> 990 set_opt (sbi->s_mount_opt, MINIX_DF);
> 991 break;
> 992 case Opt_grpid:
> 993 set_opt (sbi->s_mount_opt, GRPID);
> 955 #endif
> 956
> 957 static int parse_options (char *options, struct super_block *sb,
> 958 unsigned int *inum, unsigned long *journal_devnum,
> 959 ext3_fsblk_t *n_blocks_count, int is_remount)
> 960 {
> 961 struct ext3_sb_info *sbi = EXT3_SB(sb);
> 962 char * p;
> 963 substring_t args[MAX_OPT_ARGS];
> 964 int data_opt = 0;
> 965 int option;
> 966 kuid_t uid;
> 967 kgid_t gid;
> 968 #ifdef CONFIG_QUOTA
> 969 int qfmt;
> 970 #endif
> 971
> 972 if (!options)
> 973 return 1;
> 974
> 975 while ((p = strsep (&options, ",")) != NULL) {
> 976 int token;
> 977 if (!*p)
> 978 continue;
> 979 /*
> 980 * Initialize args struct so we know whether arg was
> 981 * found; some options take optional arguments.
> 982 */
> 983 args[0].to = args[0].from = NULL;
> 984 token = match_token(p, tokens, args);
> 985 switch (token) {
> 986 case Opt_bsd_df:
> 987 clear_opt (sbi->s_mount_opt, MINIX_DF);
> 988 break;
> 989 case Opt_minix_df:
> 990 set_opt (sbi->s_mount_opt, MINIX_DF);
> 991 break;
> 992 case Opt_grpid:
> 993 set_opt (sbi->s_mount_opt, GRPID);
> 994 break;
> 995 case Opt_nogrpid:
> 996 clear_opt (sbi->s_mount_opt, GRPID);
> 997 break;
> 998 case Opt_resuid:
> 999 if (match_int(&args[0], &option))
> 1000 return 0;
> 1001 uid = make_kuid(current_user_ns(), option);
> 1002 if (!uid_valid(uid)) {
> 1003 ext3_msg(sb, KERN_ERR, "Invalid uid value %d", option);
> 1004 return 0;
> 1005
> 1006 }
> 1007 sbi->s_resuid = uid;
> 1008 break;
> 1009 case Opt_resgid:
> 1010 if (match_int(&args[0], &option))
> 1011 return 0;
> 1012 gid = make_kgid(current_user_ns(), option);
> 1013 if (!gid_valid(gid)) {
> 1014 ext3_msg(sb, KERN_ERR, "Invalid gid value %d", option);
> 1015 return 0;
> 1016 }
> 1017 sbi->s_resgid = gid;
> 1018 break;
> 1019 case Opt_sb:
> 1020 /* handled by get_sb_block() instead of here */
> 1021 /* *sb_block = match_int(&args[0]); */
> 1022 break;
> 1023 case Opt_err_panic:
> 1024 clear_opt (sbi->s_mount_opt, ERRORS_CONT);
> 1025 clear_opt (sbi->s_mount_opt, ERRORS_RO);
> 1026 set_opt (sbi->s_mount_opt, ERRORS_PANIC);
> 1027 break;
> 1028 case Opt_err_ro:
> 1029 clear_opt (sbi->s_mount_opt, ERRORS_CONT);
> 1030 clear_opt (sbi->s_mount_opt, ERRORS_PANIC);
> 1031 set_opt (sbi->s_mount_opt, ERRORS_RO);
> 1032 break;
> 1033 case Opt_err_cont:
> 1034 clear_opt (sbi->s_mount_opt, ERRORS_RO);
> 1035 clear_opt (sbi->s_mount_opt, ERRORS_PANIC);
> 1036 set_opt (sbi->s_mount_opt, ERRORS_CONT);
> 1037 break;
> 1038 case Opt_nouid32:
> 1039 set_opt (sbi->s_mount_opt, NO_UID32);
> 1040 break;
> 1041 case Opt_nocheck:
> 1042 clear_opt (sbi->s_mount_opt, CHECK);
> 1043 break;
> 1044 case Opt_debug:
> 1045 set_opt (sbi->s_mount_opt, DEBUG);
> 1046 break;
> 1047 case Opt_oldalloc:
> 1048 ext3_msg(sb, KERN_WARNING,
> 1049 "Ignoring deprecated oldalloc option");
> 1050 break;
> 1051 case Opt_orlov:
> 1052 ext3_msg(sb, KERN_WARNING,
> 1053 "Ignoring deprecated orlov option");
> 1054 break;
> 1055 #ifdef CONFIG_EXT3_FS_XATTR
> 1056 case Opt_user_xattr:
> 1057 set_opt (sbi->s_mount_opt, XATTR_USER);
> 1058 break;
> 1059 case Opt_nouser_xattr:
> 1060 clear_opt (sbi->s_mount_opt, XATTR_USER);
> 1061 break;
> 1062 #else
> 1063 case Opt_user_xattr:
> 1064 case Opt_nouser_xattr:
> 1065 ext3_msg(sb, KERN_INFO,
> 1066 "(no)user_xattr options not supported");
> 1067 break;
> 1068 #endif
> 1069 #ifdef CONFIG_EXT3_FS_POSIX_ACL
> 1070 case Opt_acl:
> 1071 set_opt(sbi->s_mount_opt, POSIX_ACL);
> 1072 break;
> 1073 case Opt_noacl:
> 1074 clear_opt(sbi->s_mount_opt, POSIX_ACL);
> 1075 break;
> 1076 #else
> 1077 case Opt_acl:
> 1078 case Opt_noacl:
> 1079 ext3_msg(sb, KERN_INFO,
> 1080 "(no)acl options not supported");
> 1081 break;
> 1082 #endif
> 1083 case Opt_reservation:
> 1084 set_opt(sbi->s_mount_opt, RESERVATION);
> 1085 break;
> 1086 case Opt_noreservation:
> 1087 clear_opt(sbi->s_mount_opt, RESERVATION);
> 1088 break;
> 1089 case Opt_journal_update:
> 1090 /* @@@ FIXME */
> 1091 /* Eventually we will want to be able to create
> 1092 a journal file here. For now, only allow the
> 1093 user to specify an existing inode to be the
> 1094 journal file. */
> 1095 if (is_remount) {
> 1096 ext3_msg(sb, KERN_ERR, "error: cannot specify "
> 1097 "journal on remount");
> 1098 return 0;
> 1099 }
> 1100 set_opt (sbi->s_mount_opt, UPDATE_JOURNAL);
> 1101 break;
> 1102 case Opt_journal_inum:
> 1103 if (is_remount) {
> 1104 ext3_msg(sb, KERN_ERR, "error: cannot specify "
> 1105 "journal on remount");
> 1106 return 0;
> 1107 }
> 1108 if (match_int(&args[0], &option))
> 1109 return 0;
> 1110 *inum = option;
> 1111 break;
> 1112 case Opt_journal_dev:
> 1113 if (is_remount) {
> 1114 ext3_msg(sb, KERN_ERR, "error: cannot specify "
> 1115 "journal on remount");
> 1116 return 0;
> 1117 }
> 1118 if (match_int(&args[0], &option))
> 1119 return 0;
> 1120 *journal_devnum = option;
> 1121 break;
> 1122 case Opt_noload:
> 1123 set_opt (sbi->s_mount_opt, NOLOAD);
> 1124 break;
> 1125 case Opt_commit:
> 1126 if (match_int(&args[0], &option))
> 1127 return 0;
> 1128 if (option < 0)
> 1129 return 0;
> 1130 if (option == 0)
> 1131 option = JBD_DEFAULT_MAX_COMMIT_AGE;
> 1132 sbi->s_commit_interval = HZ * option;
> 1133 break;
> 1134 case Opt_data_journal:
> 1135 data_opt = EXT3_MOUNT_JOURNAL_DATA;
> 1136 goto datacheck;
> 1137 case Opt_data_ordered:
> 1138 data_opt = EXT3_MOUNT_ORDERED_DATA;
> 1139 goto datacheck;
> 1140 case Opt_data_writeback:
> 1141 data_opt = EXT3_MOUNT_WRITEBACK_DATA;
> 1142 datacheck:
> 1143 if (is_remount) {
> 1144 if (test_opt(sb, DATA_FLAGS) == data_opt)
> 1145 break;
> 1146 ext3_msg(sb, KERN_ERR,
> 1147 "error: cannot change "
> 1148 "data mode on remount. The filesystem "
> 1149 "is mounted in data=%s mode and you "
> 1150 "try to remount it in data=%s mode.",
> 1151 data_mode_string(test_opt(sb,
> 1152 DATA_FLAGS)),
> 1153 data_mode_string(data_opt));
> 1154 return 0;
> 1155 } else {
> 1156 clear_opt(sbi->s_mount_opt, DATA_FLAGS);
> 1157 sbi->s_mount_opt |= data_opt;
> 1158 }
> 1159 break;
> 1160 case Opt_data_err_abort:
> 1161 set_opt(sbi->s_mount_opt, DATA_ERR_ABORT);
> 1162 break;
> 1163 case Opt_data_err_ignore:
> 1164 clear_opt(sbi->s_mount_opt, DATA_ERR_ABORT);
> 1165 break;
> 1166 #ifdef CONFIG_QUOTA
> 1167 case Opt_usrjquota:
> 1168 if (!set_qf_name(sb, USRQUOTA, &args[0]))
> 1169 return 0;
> 1170 break;
> 1171 case Opt_grpjquota:
> 1172 if (!set_qf_name(sb, GRPQUOTA, &args[0]))
> 1173 return 0;
> 1174 break;
> 1175 case Opt_offusrjquota:
> 1176 if (!clear_qf_name(sb, USRQUOTA))
> 1177 return 0;
> 1178 break;
> 1179 case Opt_offgrpjquota:
> 1180 if (!clear_qf_name(sb, GRPQUOTA))
> 1181 return 0;
> 1182 break;
>
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2012-12-27 8:25 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-26 5:04 [Suggestion] fs/ext3: memory leak by calling set_qf_name or clear_qf_name, many times Chen Gang
2012-12-27 8:06 ` Chen Gang [this message]
2012-12-31 12:06 ` Jan Kara
2013-01-04 5:39 ` Chen Gang
2013-01-07 19:54 ` Jan Kara
2013-01-08 3:13 ` Chen Gang
2013-01-14 2:29 ` Chen Gang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50DC016A.9070801@asianux.com \
--to=gang.chen@asianux.com \
--cc=adilger.kernel@dilger.ca \
--cc=akpm@linux-foundation.org \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.