linux-kernel - [RFC 1/5] security: smack: avoid kmalloc allocations while loading a rule string

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <1371137352-31273-2-git-send-email-t.stanislaws@samsung.com>
Date:	Thu, 13 Jun 2013 17:29:08 +0200
From:	Tomasz Stanislawski <t.stanislaws@...sung.com>
To:	linux-security-module@...r.kernel.org
Cc:	m.szyprowski@...sung.com, kyungmin.park@...sung.com,
	r.krypa@...sung.com, linux-kernel@...r.kernel.org,
	casey@...aufler-ca.com,
	Tomasz Stanislawski <t.stanislaws@...sung.com>
Subject: [RFC 1/5] security: smack: avoid kmalloc allocations while loading a
 rule string

The maximal length for a rule line for long format is introduced as
SMK_LOAD2LEN. This allows a buffer for a rule string to be allocated
on a stack instead of a heap (aka kmalloc cache).

Limiting the length of a rule line helps to avoid allocations of a very long
contiguous buffer from a heap if user calls write() for a very long chunk.
Such an allocation often causes a lot swapper/writeback havoc and it is very
likely to fails.

Moreover, stack allocation is slightly faster than from kmalloc.

Signed-off-by: Tomasz Stanislawski <t.stanislaws@...sung.com>
---
 security/smack/smackfs.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 53a08b8..9a3cd0d 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -137,6 +137,7 @@ const char *smack_cipso_option = SMACK_CIPSO_OPTION;
  * SMK_ACCESS: Maximum possible combination of access permissions
  * SMK_ACCESSLEN: Maximum length for a rule access field
  * SMK_LOADLEN: Smack rule length
+ * SMK_LOAD2LEN: Smack maximal long rule length excluding \0
  */
 #define SMK_OACCESS	"rwxa"
 #define SMK_ACCESS	"rwxat"
@@ -144,6 +145,7 @@ const char *smack_cipso_option = SMACK_CIPSO_OPTION;
 #define SMK_ACCESSLEN	(sizeof(SMK_ACCESS) - 1)
 #define SMK_OLOADLEN	(SMK_LABELLEN + SMK_LABELLEN + SMK_OACCESSLEN)
 #define SMK_LOADLEN	(SMK_LABELLEN + SMK_LABELLEN + SMK_ACCESSLEN)
+#define SMK_LOAD2LEN	(2 * SMK_LONGLABEL + SMK_ACCESSLEN + 2)
 
 /*
  * Stricly for CIPSO level manipulation.
@@ -447,8 +449,7 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
 {
 	struct smack_known *skp;
 	struct smack_parsed_rule *rule;
-	char *data;
-	int datalen;
+	char data[SMK_LOAD2LEN + 1];
 	int rc = -EINVAL;
 	int load = 0;
 
@@ -465,13 +466,10 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
 		 */
 		if (count != SMK_OLOADLEN && count != SMK_LOADLEN)
 			return -EINVAL;
-		datalen = SMK_LOADLEN;
-	} else
-		datalen = count + 1;
+	}
 
-	data = kzalloc(datalen, GFP_KERNEL);
-	if (data == NULL)
-		return -ENOMEM;
+	if (count > SMK_LOAD2LEN)
+		count = SMK_LOAD2LEN;
 
 	if (copy_from_user(data, buf, count) != 0) {
 		rc = -EFAULT;
@@ -522,7 +520,6 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
 out_free_rule:
 	kfree(rule);
 out:
-	kfree(data);
 	return rc;
 }
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/