Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1435353 > unrolled thread

[PATCH v2] kasan/quarantine: fix bugs on qlist_move_cache()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2] kasan/quarantine: fix bugs on qlist_move_cache() js1304@gmail.com - 2016-07-01 16:00 +0200
    Re: [PATCH v2] kasan/quarantine: fix bugs on qlist_move_cache() Joonsoo Kim <js1304@gmail.com> - 2016-07-01 16:10 +0200
    Re: [PATCH v2] kasan/quarantine: fix bugs on qlist_move_cache() kbuild test robot <lkp@intel.com> - 2016-07-02 10:50 +0200

#1435353 — [PATCH v2] kasan/quarantine: fix bugs on qlist_move_cache()

From: Joonsoo Kim <iamjoonsoo.kim@lge.com>

There are two bugs on qlist_move_cache(). One is that qlist's tail
isn't set properly. curr->next can be NULL since it is singly linked
list and NULL value on tail is invalid if there is one item on qlist.
Another one is that if cache is matched, qlist_put() is called and
it will set curr->next to NULL. It would cause to stop the loop
prematurely.

These problems come from complicated implementation so I'd like to
re-implement it completely. Implementation in this patch is really
simple. Iterate all qlist_nodes and put them to appropriate list.

Unfortunately, I got this bug sometime ago and lose oops message.
But, the bug looks trivial and no need to attach oops.

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
 mm/kasan/quarantine.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c
index 4973505..061d39b 100644
--- a/mm/kasan/quarantine.c
+++ b/mm/kasan/quarantine.c
@@ -238,30 +238,24 @@ static void qlist_move_cache(struct qlist_head *from,
 				   struct qlist_head *to,
 				   struct kmem_cache *cache)
 {
-	struct qlist_node *prev = NULL, *curr;
+	struct qlist_node *curr;
+	struct qlist_node *head = NULL, *tail = NULL;
 
 	if (unlikely(qlist_empty(from)))
 		return;
 
 	curr = from->head;
+	qlist_init(from);
 	while (curr) {
 		struct qlist_node *qlink = curr;
 		struct kmem_cache *obj_cache = qlink_to_cache(qlink);
 
-		if (obj_cache == cache) {
-			if (unlikely(from->head == qlink)) {
-				from->head = curr->next;
-				prev = curr;
-			} else
-				prev->next = curr->next;
-			if (unlikely(from->tail == qlink))
-				from->tail = curr->next;
-			from->bytes -= cache->size;
-			qlist_put(to, qlink, cache->size);
-		} else {
-			prev = curr;
-		}
 		curr = curr->next;
+
+		if (obj_cache == cache)
+			qlist_put(to, qlink, cache->size);
+		else
+			qlist_put(from, qlink, cache->size);
 	}
 }
 
-- 
1.9.1

[toc] | [next] | [standalone]

#1435359

2016-07-01 22:55 GMT+09:00  <js1304@gmail.com>:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> There are two bugs on qlist_move_cache(). One is that qlist's tail
> isn't set properly. curr->next can be NULL since it is singly linked
> list and NULL value on tail is invalid if there is one item on qlist.
> Another one is that if cache is matched, qlist_put() is called and
> it will set curr->next to NULL. It would cause to stop the loop
> prematurely.
>
> These problems come from complicated implementation so I'd like to
> re-implement it completely. Implementation in this patch is really
> simple. Iterate all qlist_nodes and put them to appropriate list.
>
> Unfortunately, I got this bug sometime ago and lose oops message.
> But, the bug looks trivial and no need to attach oops.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Please ignore this. It causes build warning. Please see v3.
Sorry for noise.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1435749

[Multipart message — attachments visible in raw view] — view raw

Hi,

[auto build test WARNING on v4.7-rc5]
[also build test WARNING on next-20160701]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/js1304-gmail-com/kasan-quarantine-fix-bugs-on-qlist_move_cache/20160702-102811
config: x86_64-randconfig-r0-07021543 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   mm/kasan/quarantine.c: In function 'qlist_move_cache':
>> mm/kasan/quarantine.c:242:35: warning: unused variable 'tail' [-Wunused-variable]
     struct qlist_node *head = NULL, *tail = NULL;
                                      ^~~~
>> mm/kasan/quarantine.c:242:21: warning: unused variable 'head' [-Wunused-variable]
     struct qlist_node *head = NULL, *tail = NULL;
                        ^~~~

vim +/tail +242 mm/kasan/quarantine.c

   226			    global_quarantine.bytes - QUARANTINE_LOW_SIZE)
   227				break;
   228			last = last->next;
   229		}
   230		qlist_move(&global_quarantine, last, &to_free, size_to_free);
   231	
   232		spin_unlock_irqrestore(&quarantine_lock, flags);
   233	
   234		qlist_free_all(&to_free, NULL);
   235	}
   236	
   237	static void qlist_move_cache(struct qlist_head *from,
   238					   struct qlist_head *to,
   239					   struct kmem_cache *cache)
   240	{
   241		struct qlist_node *curr;
 > 242		struct qlist_node *head = NULL, *tail = NULL;
   243	
   244		if (unlikely(qlist_empty(from)))
   245			return;
   246	
   247		curr = from->head;
   248		qlist_init(from);
   249		while (curr) {
   250			struct qlist_node *qlink = curr;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web