Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Wed, 11 May 2022 15:32:29 -0700
From:   Kees Cook <keescook@chromium.org>
To:     Maninder Singh <maninder1.s@samsung.com>
Cc:     mcgrof@kernel.org, avimalin@gmail.com, ast@kernel.org,
        daniel@iogearbox.net, andrii@kernel.org, kafai@fb.com,
        songliubraving@fb.com, yhs@fb.com, john.fastabend@gmail.com,
        kpsingh@kernel.org, pmladek@suse.com, rostedt@goodmis.org,
        senozhatsky@chromium.org, andriy.shevchenko@linux.intel.com,
        naveen.n.rao@linux.ibm.com, davem@davemloft.net,
        mhiramat@kernel.org, anil.s.keshavamurthy@intel.com,
        linux@rasmusvillemoes.dk, akpm@linux-foundation.org,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        v.narang@samsung.com, Onkarnath <onkarnath.1@samsung.com>
Subject: Re: [PATCH 1/2] kallsyms: add kallsyms_show_value definition in all
 cases
Message-ID: <202205111525.92B1C597@keescook>
References: <CGME20220511080722epcas5p459493d02ff662a7c75590e44a11e34a6@epcas5p4.samsung.com>
 <20220511080657.3996053-1-maninder1.s@samsung.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220511080657.3996053-1-maninder1.s@samsung.com>
Precedence: bulk

On Wed, May 11, 2022 at 01:36:56PM +0530, Maninder Singh wrote:
> kallsyms_show_value return false if KALLSYMS is disabled,
> but its usage is done by module.c also.
> Thus when KALLSYMS is disabled, system will not print module
> load address:

Eek, I hadn't see the other changes this depends on. I think those
changes need to be reworked first. Notably in the other patch, this is
no good:

        /* address belongs to module */
        if (add_offset)
                len = sprintf(buf, "0x%p+0x%lx", base, offset);
        else
                len = sprintf(buf, "0x%lx", value);

This is printing raw kernel addresses with no hashing, as far as I can
tell. That's not okay at all.

Once that other patch gets fixed, this one then can be revisited.

And just on naming: "kallsyms_tiny" is a weird name: it's just "ksyms"
-- there's no "all".  :)

-- 
Kees Cook