linux-kernel - Re: [PATCH v12 2/3] ipc: Conserve sequence numbers in ipcmni

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: <c66181f7-e19e-0764-9de7-6bcefaef398a@redhat.com>
Date:   Mon, 18 Mar 2019 14:57:47 -0400
From:   Waiman Long <longman@...hat.com>
To:     Manfred Spraul <manfred@...orfullife.com>,
        "Luis R. Rodriguez" <mcgrof@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jonathan Corbet <corbet@....net>
Cc:     linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-doc@...r.kernel.org, Al Viro <viro@...iv.linux.org.uk>,
        Matthew Wilcox <willy@...radead.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Takashi Iwai <tiwai@...e.de>, Davidlohr Bueso <dbueso@...e.de>,
        1vier1@....de
Subject: Re: [PATCH v12 2/3] ipc: Conserve sequence numbers in ipcmni_extend
 mode

On 03/16/2019 02:52 PM, Manfred Spraul wrote:
> Hi,
>
> On 2/28/19 7:47 PM, Waiman Long wrote:
>> @@ -216,10 +221,11 @@ static inline int ipc_idr_alloc(struct ipc_ids
>> *ids, struct kern_ipc_perm *new)
>>        */
>>         if (next_id < 0) { /* !CHECKPOINT_RESTORE or next_id is unset */
>> -        new->seq = ids->seq++;
>> -        if (ids->seq > IPCID_SEQ_MAX)
>> -            ids->seq = 0;
>>           idx = idr_alloc(&ids->ipcs_idr, new, 0, 0, GFP_NOWAIT);
>> +        if ((idx <= ids->last_idx) && (++ids->seq > IPCID_SEQ_MAX))
>> +            ids->seq = 0;
>
> I'm always impressed by such lines:
>
> Everything in just two lines, use "++a", etc.
>
> But: How did you test it?
>
> idr_alloc() can fail, the code doesn't handle that :-(
>
>

You are right. I should have checked for the error case.

Thanks for spotting that.

Cheers,
Longman