linux-kernel - Re: [PATCH v2] powerpc/mm: move a KERN_WARNING message to pr

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: <632657bb-acd7-54bd-ac02-0d6f9d87d26b@redhat.com>
Date:   Thu, 7 Feb 2019 10:25:09 +0100
From:   Laurent Vivier <lvivier@...hat.com>
To:     David Gibson <david@...son.dropbear.id.au>
Cc:     linux-kernel@...r.kernel.org,
        Michael Ellerman <mpe@...erman.id.au>,
        linuxppc-dev@...ts.ozlabs.org,
        Christophe Leroy <christophe.leroy@....fr>
Subject: Re: [PATCH v2] powerpc/mm: move a KERN_WARNING message to pr_debug()

On 07/02/2019 04:03, David Gibson wrote:
> On Tue, Feb 05, 2019 at 09:21:33PM +0100, Laurent Vivier wrote:
>> resize_hpt_for_hotplug() reports a warning when it cannot
>> increase the hash page table ("Unable to resize hash page
>> table to target order") but this is not blocking and
>> can make user thinks something has not worked properly.
>> As we move the message to the debug area, report again the
>> ENODEV error.
>>
>> If the operation cannot be done the real error message
>> will be reported by arch_add_memory() if create_section_mapping()
>> fails.
>>
>> Fixes: 7339390d772dd
>>        powerpc/pseries: Don't give a warning when HPT resizing isn't available
>> Signed-off-by: Laurent Vivier <lvivier@...hat.com>
> 
> Sorry, I'm pretty dubious about this.  It's true that in the case for
> which this bug was filed this is a harmless situation which deserves a
> pr_debug() at most.
> 
> But that's not necessarily true in all paths leading to this message.
> It will also trip if we fail to reshrink the HPT after genuinely
> hotunplugging a bunch of memory, in which case failing to release
> expected resources does deserve a warning.

But if there is a real problem this function should return an error and
this error should be managed by the caller.

Moreover, the function that can fail (pseries_lpar_resize_hpt()) has
already a warning for each error case:

ETIMEDOUT: "Unexpected error %d cancelling timed out HPT resize\n"
EIO:       "Unexpected error %d from H_RESIZE_HPT_PREPARE\n"
           "Unexpected error %d from H_RESIZE_HPT_COMMIT\n
ENOSPC:    "Hash collision while resizing HPT\n"

ENODEV has no error message but is already silently ignored.
EINVAL and EPERM have no message but this happens if hcall is not used
correctly and deserve only a pr_debug() I think.

Thanks,
Laurent