linux-kernel - Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate

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] [day] [month] [year] [list]
Message-ID: <87iowhv7nq.fsf@redhat.com>
Date:	Mon, 28 Oct 2013 21:36:57 +0800
From:	Madper Xie <cxie@...hat.com>
To:	Tony Luck <tony.luck@...il.com>
Cc:	Anton Vorontsov <anton@...msg.org>,
	"linux-efi\@vger.kernel.org" <linux-efi@...r.kernel.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
	Seiji Aguchi <seiji.aguchi@....com>,
	Lenny Szubowicz <lszubowi@...hat.com>,
	成骏 谢 <bbboson@...il.com>,
	Colin Cross <ccross@...roid.com>,
	Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate

Howdy Tony,
  Does this patch still need some rework?
  
cxie@...hat.com writes:

> tony.luck@...il.com writes:
>
>> On Wed, Oct 23, 2013 at 7:55 AM, Madper Xie <cxie@...hat.com> wrote:
>>> The "duplicate" entries won't appear in pstorefs. And a complain will be
>>> print -- pstore: failed to load 76 record(s) from 'efi'
>>
>> Maybe I don't quite get this - but it sounds like you have a whole lot
>> of entries using up space in efivars that have similar names - differing
>> just in the timestamp - that won't show up in the pstore filesystem - because
>> we'd try to name them all the same thing.
>>
> Maybe I misunderstand you...
>
> Sure pstore try to name them all the same thing, but it's another
> issue. and it doesn't prevent entries showing up in pstore fs.
>
> Consider the following case: (after efi-pstore support append mode, it
> always like this case):
>
> I choose four dumped efivars from my DELL XPS:
> dump-type0-9-1-1380441690-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
> dump-type0-9-1-1380448560-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
> dump-type0-9-1-1380460890-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
> dump-type0-9-1-1382496073-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
>          ^        ^       [              ^                   ]
>          !        !                      !
>        type    timestamp                GUID
>
> When pstore load them from efivars, pstore incorrectly assuming that
> efivars with the same TYPE, ID and GUID are duplicate.
>
> list_for_each_entry(pos, &allpstore, list) {
> 	if (pos->type == type &&    /---
> 	    pos->id == id &&     <-| as I said, it only check type,id,psi
> 	    pos->psi == psi) {      \---
> 		rc = -EEXIST;    <- then set -EEXIST, and ignore *dup* entry
> 		break;
> 	}
> }
> You can see the code above, for those four entries, only one could be
> showed in pstorefs, all others will get a -EEXIST. So I add a timestamp
> check here, it's the only different part.
>
>> How did all those things end up in efivars?
> before the patch I can see 
> dmesg-efi-1   dmesg-efi-11  dmesg-efi-3  dmesg-efi-5  dmesg-efi-7  dmesg-efi-9
> dmesg-efi-10  dmesg-efi-2   dmesg-efi-4  dmesg-efi-6  dmesg-efi-8
> after apply the patch:
> [root@...p-13-41 vars]# ls /dev/pstore/
> dmesg-efi-1  dmesg-efi-1   dmesg-efi-11  dmesg-efi-2  dmesg-efi-3  dmesg-efi-5  dmesg-efi-6  dmesg-efi-8
> dmesg-efi-1  dmesg-efi-10  dmesg-efi-11  dmesg-efi-2  dmesg-efi-3  dmesg-efi-5  dmesg-efi-7  dmesg-efi-8
> dmesg-efi-1  dmesg-efi-10  dmesg-efi-12  dmesg-efi-2  dmesg-efi-4  dmesg-efi-5  dmesg-efi-7  dmesg-efi-8
> dmesg-efi-1  dmesg-efi-10  dmesg-efi-13  dmesg-efi-2  dmesg-efi-4  dmesg-efi-5  dmesg-efi-7  dmesg-efi-9
> dmesg-efi-1  dmesg-efi-10  dmesg-efi-14  dmesg-efi-2  dmesg-efi-4  dmesg-efi-5  dmesg-efi-7  dmesg-efi-9
> dmesg-efi-1  dmesg-efi-10  dmesg-efi-15  dmesg-efi-2  dmesg-efi-4  dmesg-efi-5  dmesg-efi-7  dmesg-efi-9
> dmesg-efi-1  dmesg-efi-10  dmesg-efi-16  dmesg-efi-3  dmesg-efi-4  dmesg-efi-5  dmesg-efi-7  dmesg-efi-9
> dmesg-efi-1  dmesg-efi-10  dmesg-efi-2   dmesg-efi-3  dmesg-efi-4  dmesg-efi-6  dmesg-efi-7  dmesg-efi-9
> dmesg-efi-1  dmesg-efi-10  dmesg-efi-2   dmesg-efi-3  dmesg-efi-4  dmesg-efi-6  dmesg-efi-7  dmesg-efi-9
> dmesg-efi-1  dmesg-efi-10  dmesg-efi-2   dmesg-efi-3  dmesg-efi-4  dmesg-efi-6  dmesg-efi-8  dmesg-efi-9
> dmesg-efi-1  dmesg-efi-11  dmesg-efi-2   dmesg-efi-3  dmesg-efi-4  dmesg-efi-6  dmesg-efi-8  dmesg-efi-9
> dmesg-efi-1  dmesg-efi-11  dmesg-efi-2   dmesg-efi-3  dmesg-efi-4  dmesg-efi-6  dmesg-efi-8  dmesg-efi-9
> dmesg-efi-1  dmesg-efi-11  dmesg-efi-2   dmesg-efi-3  dmesg-efi-5  dmesg-efi-6  dmesg-efi-8
> dmesg-efi-1  dmesg-efi-11  dmesg-efi-2   dmesg-efi-3  dmesg-efi-5  dmesg-efi-6  dmesg-efi-8
> dmesg-efi-1  dmesg-efi-11  dmesg-efi-2   dmesg-efi-3  dmesg-efi-5  dmesg-efi-6  dmesg-efi-8
>
>>
>> Wouldn't the right fix be to make pstore allow them all to appear (using the
>> timestamp to differentiate names?) so that we could see them, log them,
>> and then remove them from pstore (in turn freeing up efivars space - which
>> people keep telling me is in short supply).
>>
> Yeah, many file have the same name, just like my case above. But it not
> really block the file shows up and should be solved in another
> patch. And I'm trying fix it.
>> -Tony


-- 
Best,
Madper Xie.
--
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/