Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 20 Oct 2022 16:57:22 +0300
From:   Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To:     Petr Mladek <pmladek@suse.com>
Cc:     Jane Chu <jane.chu@oracle.com>,
        "rostedt@goodmis.org" <rostedt@goodmis.org>,
        "senozhatsky@chromium.org" <senozhatsky@chromium.org>,
        "linux@rasmusvillemoes.dk" <linux@rasmusvillemoes.dk>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        Haakon Bugge <haakon.bugge@oracle.com>,
        John Haxby <john.haxby@oracle.com>
Subject: Re: [PATCH] vsprintf: protect kernel from panic due to non-canonical
 pointer dereference
Message-ID: <Y1FTwpngS5EPgSX4@smile.fi.intel.com>
References: <20221017191611.2577466-1-jane.chu@oracle.com>
 <Y02sENwhtpsx5yhP@smile.fi.intel.com>
 <5d987403-a7bf-8996-d639-c99edeaabcdf@oracle.com>
 <Y06f4EwisLTU0rEz@alley>
 <799e5390-2ff5-02b7-2df7-61198d5451e2@oracle.com>
 <Y08Hn6on37fgc57F@smile.fi.intel.com>
 <f77145e0-2bc9-a558-7f55-22100fa4b5ed@oracle.com>
 <Y08RVzvniYr8ycKV@smile.fi.intel.com>
 <Y1D8RfpkyLa4FU7r@alley>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y1D8RfpkyLa4FU7r@alley>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

On Thu, Oct 20, 2022 at 09:44:05AM +0200, Petr Mladek wrote:
> On Tue 2022-10-18 23:49:27, Andy Shevchenko wrote:
> > On Tue, Oct 18, 2022 at 08:30:01PM +0000, Jane Chu wrote:

...

> > Obviously, to see the crash. And let kernel _to crash_. Isn't it what we need
> > to see a bug as early as possible?
> 
> I do not agree here. Kernel tries to survive many situations when
> thighs does not work as expected. It prints a warning so that
> users/developers are aware of the problem and could fix it.

How the user will know what the root cause and how to fix it? The crash
report will give all needed information, the "(eXXXXXX)" will hide it all,
which I consider inappropriate approach.

I.o.w. consider "(eXXXXXX)" vs. something like "your stuff crashed kernel
because of misaligned / etc pointer which has value of 0xXXXXXXXX and other
registers have these values" and so on, so on...

> In our case, the crash happened when reading a sysfs file.
> IMHO, it is much better to show (-EINVAL) than crash. The bug
> when accessing devX_attrY[] does not affect the stability of
> the system at all.

When I got "eXXXXX" from cat /sys/... I think "OK, something went wrong,
I shouldn't really take it seriously". And completely different feelings
when you got a crash, right?

> And the broken string might be passed in a very rare case,
> e.g. in an error path. So that it might be hard to catch
> when testing.

-- 
With Best Regards,
Andy Shevchenko