mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tomasz Stanislawski <t.stanislaws@samsung.com>
To: linux-security-module@vger.kernel.org
Cc: m.szyprowski@samsung.com, kyungmin.park@samsung.com,
	r.krypa@samsung.com, linux-kernel@vger.kernel.org,
	casey@schaufler-ca.com,
	Tomasz Stanislawski <t.stanislaws@samsung.com>
Subject: [RFC 2/5] security: smack: avoid kmalloc() in smk_parse_long_rule()
Date: Thu, 13 Jun 2013 17:29:09 +0200	[thread overview]
Message-ID: <1371137352-31273-3-git-send-email-t.stanislaws@samsung.com> (raw)
In-Reply-To: <1371137352-31273-1-git-send-email-t.stanislaws@samsung.com>

Function smk_parse_long_rule() allocates a number of temporary strings on heap
(kmalloc cache). Moreover, the sizes of those allocations might be large if
user calls write() for a long chunk. A big kmalloc triggers a heavy reclaim
havoc and it is very likely to fail.

This patch introduces smk_parse_substrings() function that parses a string into
substring separated by whitespaces.  The buffer for substring is preallocated.
It must store substring the worst case scenario which is SMK_LOAD2LEN in case
of long rule parsing.

The buffer is allocated on stack what is slightly faster than kmalloc().

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
---
 security/smack/smackfs.c |   67 +++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 9a3cd0d..46f111e 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -364,6 +364,33 @@ static int smk_parse_rule(const char *data, struct smack_parsed_rule *rule,
 }
 
 /**
+ * smk_parse_strings - parse white-space separated substring from a string
+ * @src: a long string to be parsed, null terminated
+ * @dst: a buffer for substrings, should be at least strlen(src)+1 bytes
+ * @args: table for parsed substring
+ * @size: number of slots in args table
+ *
+ * Returns number of parsed substrings
+ */
+static int smk_parse_substrings(const char *src, char *dst,
+	char *args[], int size)
+{
+	int cnt;
+
+	for (cnt = 0; cnt < size; ++cnt) {
+		src = skip_spaces(src);
+		if (*src == 0)
+			break;
+		args[cnt] = dst;
+		for (; *src && !isspace(*src); ++src, ++dst)
+			*dst = *src;
+		*(dst++) = 0;
+	}
+
+	return cnt;
+}
+
+/**
  * smk_parse_long_rule - parse Smack rule from rule string
  * @data: string to be parsed, null terminated
  * @rule: Will be filled with Smack parsed rule
@@ -375,48 +402,20 @@ static int smk_parse_rule(const char *data, struct smack_parsed_rule *rule,
 static int smk_parse_long_rule(const char *data, struct smack_parsed_rule *rule,
 				int import, int change)
 {
-	char *subject;
-	char *object;
-	char *access1;
-	char *access2;
-	int datalen;
+	char tmp[SMK_LOAD2LEN + 1];
+	char *args[4];
 	int rc = -1;
 
-	/* This is inefficient */
-	datalen = strlen(data);
-
-	/* Our first element can be 64 + \0 with no spaces */
-	subject = kzalloc(datalen + 1, GFP_KERNEL);
-	if (subject == NULL)
-		return -1;
-	object = kzalloc(datalen, GFP_KERNEL);
-	if (object == NULL)
-		goto free_out_s;
-	access1 = kzalloc(datalen, GFP_KERNEL);
-	if (access1 == NULL)
-		goto free_out_o;
-	access2 = kzalloc(datalen, GFP_KERNEL);
-	if (access2 == NULL)
-		goto free_out_a;
-
 	if (change) {
-		if (sscanf(data, "%s %s %s %s",
-			subject, object, access1, access2) == 4)
-			rc = smk_fill_rule(subject, object, access1, access2,
+		if (smk_parse_substrings(data, tmp, args, 4) == 4)
+			rc = smk_fill_rule(args[0], args[1], args[2], args[3],
 				rule, import, 0);
 	} else {
-		if (sscanf(data, "%s %s %s", subject, object, access1) == 3)
-			rc = smk_fill_rule(subject, object, access1, NULL,
+		if (smk_parse_substrings(data, tmp, args, 3) == 3)
+			rc = smk_fill_rule(args[0], args[1], args[2], NULL,
 				rule, import, 0);
 	}
 
-	kfree(access2);
-free_out_a:
-	kfree(access1);
-free_out_o:
-	kfree(object);
-free_out_s:
-	kfree(subject);
 	return rc;
 }
 
-- 
1.7.9.5


  parent reply	other threads:[~2013-06-13 15:29 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-13 15:29 [RFC 0/5] Optimizations for memory handling in smk_write_rules_list() Tomasz Stanislawski
2013-06-13 15:29 ` [RFC 1/5] security: smack: avoid kmalloc allocations while loading a rule string Tomasz Stanislawski
2013-06-15 19:32   ` Casey Schaufler
2013-06-17 11:24     ` Tomasz Stanislawski
2013-06-17 22:38       ` Casey Schaufler
2013-06-13 15:29 ` Tomasz Stanislawski [this message]
2013-06-15 19:41   ` [RFC 2/5] security: smack: avoid kmalloc() in smk_parse_long_rule() Casey Schaufler
2013-06-13 15:29 ` [RFC 3/5] security: smack: fix memleak in smk_write_rules_list() Tomasz Stanislawski
2013-06-15 19:54   ` Casey Schaufler
2013-06-13 15:29 ` [RFC 4/5] security: smack: add kmem_cache for smack_rule allocations Tomasz Stanislawski
2013-06-15 20:00   ` Casey Schaufler
2013-06-13 15:29 ` [RFC 5/5] security: smack: add kmem_cache for smack_master_list allocations Tomasz Stanislawski
2013-06-15 20:08   ` Casey Schaufler
2013-06-19 14:08 ` [PATCH] security: smack: fix memleak in smk_write_rules_list() Tomasz Stanislawski
2013-06-28 19:33   ` Casey Schaufler
2013-08-01 20:01   ` Casey Schaufler

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=1371137352-31273-3-git-send-email-t.stanislaws@samsung.com \
    --to=t.stanislaws@samsung.com \
    --cc=casey@schaufler-ca.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=r.krypa@samsung.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®