All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joseph Qi <joseph.qi@huawei.com>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: kbuild test robot <fengguang.wu@intel.com>,
	kbuild-all@01.org, akpm@linux-foundation.org, linux-mm@kvack.org,
	kbuild@01.org
Subject: Re: fs/ocfs2/dlm/dlmrecovery.c:1824:4-23: iterator with update on line 1827
Date: Tue, 8 Sep 2015 16:40:58 +0800	[thread overview]
Message-ID: <55EE9F1A.6000701@huawei.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1509080916550.2342@hadrien>

On 2015/9/8 15:22, Julia Lawall wrote:
> On Tue, 8 Sep 2015, Joseph Qi wrote:
> 
>> Hi Julia,
>>
>> On 2015/9/7 22:01, Julia Lawall wrote:
>>> It looks like a serious problem, because the loop update does a
>>> dereference of the first argument of list_for_each via list_entry.
>>>
>> Could you give more details about this? IMO, it doesn't make any
>> difference in functional logic.
> 
> Do you expect that setting lock to NULL will cause a break out of the
> loop?  Because it does not.  The expansion of list_for_each_entry is:
> 
> #define list_for_each_entry(pos, head, member)                          \
> 	for (pos = list_first_entry(head, typeof(*pos), member);        \
>              &pos->member != (head);                                    \
>              pos = list_next_entry(pos, member))
> 
> Since pos is NULL, &pos->member != (head), so we will take the loop
> update. List_next_entry is:
> 
My understanding is, pos is an address rather than NULL, which is
calculated from head's address with an offset.
In case of an empty head, &pos->member will compensate the offset again
and makes it evaluated the same head. Then it breaks out the loop.

Joseph

> #define list_next_entry(pos, member) \
>         list_entry((pos)->member.next, typeof(*(pos)), member)
> 
> list_entry is container_of, which does
> 
> const typeof( ((type *)0)->member ) *__mptr = (ptr);
> 
> This causes (pos)->member.next to be evaluated, which since pos is NULL
> will crash.
> 
> julia
> 
>>
>>> julia
>>>
>>> On Mon, 7 Sep 2015, kbuild test robot wrote:
>>>
>>>> TO: Joseph Qi <joseph.qi@huawei.com>
>>>> CC: kbuild-all@01.org
>>>> CC: Andrew Morton <akpm@linux-foundation.org>
>>>> CC: Linux Memory Management List <linux-mm@kvack.org>
>>>>
>>>> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>>>> head:   7d9071a095023cd1db8fa18fa0d648dc1a5210e0
>>>> commit: f83c7b5e9fd633fe91128af116e6472a8c4d29a5 ocfs2/dlm: use list_for_each_entry instead of list_for_each
>>>> date:   3 days ago
>>>> :::::: branch date: 33 hours ago
>>>> :::::: commit date: 3 days ago
>>>>
>>>>>> fs/ocfs2/dlm/dlmrecovery.c:1824:4-23: iterator with update on line 1827
>>>>
>>>> git remote add linus git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>>>> git remote update linus
>>>> git checkout f83c7b5e9fd633fe91128af116e6472a8c4d29a5
>>>> vim +1824 fs/ocfs2/dlm/dlmrecovery.c
>>>>
>>>> 6714d8e8 Kurt Hackel 2005-12-15  1818  			BUG_ON(!(mres->flags & DLM_MRES_MIGRATION));
>>>> 6714d8e8 Kurt Hackel 2005-12-15  1819
>>>> 34aa8dac Junxiao Bi  2014-04-03  1820  			lock = NULL;
>>>> 6714d8e8 Kurt Hackel 2005-12-15  1821  			spin_lock(&res->spinlock);
>>>> e17e75ec Kurt Hackel 2007-01-05  1822  			for (j = DLM_GRANTED_LIST; j <= DLM_BLOCKED_LIST; j++) {
>>>> e17e75ec Kurt Hackel 2007-01-05  1823  				tmpq = dlm_list_idx_to_ptr(res, j);
>>>> f83c7b5e Joseph Qi   2015-09-04 @1824  				list_for_each_entry(lock, tmpq, list) {
>>>> 34aa8dac Junxiao Bi  2014-04-03  1825  					if (lock->ml.cookie == ml->cookie)
>>>> 6714d8e8 Kurt Hackel 2005-12-15  1826  						break;
>>>> 34aa8dac Junxiao Bi  2014-04-03 @1827  					lock = NULL;
>>>> 6714d8e8 Kurt Hackel 2005-12-15  1828  				}
>>>> e17e75ec Kurt Hackel 2007-01-05  1829  				if (lock)
>>>> e17e75ec Kurt Hackel 2007-01-05  1830  					break;
>>>>
>>>> ---
>>>> 0-DAY kernel test infrastructure                Open Source Technology Center
>>>> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>>>>
>>>
>>> .
>>>
>>
>>
>>
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 
> .
> 


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2015-09-08  8:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <201509072033.3vy462XZ%fengguang.wu@intel.com>
2015-09-07 14:01 ` fs/ocfs2/dlm/dlmrecovery.c:1824:4-23: iterator with update on line 1827 Julia Lawall
2015-09-08  6:13   ` Joseph Qi
2015-09-08  7:22     ` Julia Lawall
2015-09-08  8:40       ` Joseph Qi [this message]
2015-09-08  9:00         ` Julia Lawall
2015-09-08  9:26           ` Joseph Qi

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=55EE9F1A.6000701@huawei.com \
    --to=joseph.qi@huawei.com \
    --cc=akpm@linux-foundation.org \
    --cc=fengguang.wu@intel.com \
    --cc=julia.lawall@lip6.fr \
    --cc=kbuild-all@01.org \
    --cc=kbuild@01.org \
    --cc=linux-mm@kvack.org \
    /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.