Re: Accessing external code or data from the 'console' kernel module: Can it be done?

[Luc Van Rompaey]

On 29/06/15 20:53, Andrei Borzenkov wrote:

> В Mon, 29 Jun 2015 19:43:39 +0200
> Luc Van Rompaey <address@hidden> пишет:
>
> > On 26/06/15 08:08, Andrei Borzenkov wrote:
> >
> > > В Thu, 25 Jun 2015 21:23:59 +0200
> > > Luc Van Rompaey <address@hidden> пишет:
> > >
> > > > On 24/06/15 08:37, Andrei Borzenkov wrote:
> > > > > On Tue, Jun 23, 2015 at 8:35 PM, Luc Van Rompaey
> > > > > <address@hidden> wrote:
> > > > > > Make console driver export pointer to a table. This will make it
> > > > > > possible for console driver to work without your module but provides
> > > > > > access to this table if needed.
> > > > > >
> > > > > > Of course console driver will need to cope with NULL pointer (or
> > > > > > initialize table to something meaningful).
> > > > > >
> > > > > > Hmmm... I think this is the one thing that I have not yet tried.
> > > > > > If I understand correctly, I would then have to create a an EXPORT_VAR
> > > > > > pointer variable in the console module, and let my 'setkey' module store the
> > > > > > appropriate value into it.
> > > > > > The function to remap an input key, according to the contents of the table,
> > > > > > would then be a part of the console driver.
> > > > > It can also be a function pointer. So code in console driver would reduce to
> > > > >
> > > > > if (grub_setkey_xlat)
> > > > >     grub_setkey_xlat(...)
> > > > I'm afraid I cannot get this to work, even though I'm trying to mimic
> > > > some other code that appears to use the same mechanism.
> > > > Here's what I'm doing:
> > > >
> > > > In console.c (i.e., 'grub-core/term/i386/pc/console.c'), I define the
> > > > grub_setkey_xlat function pointer:
> > > >
> > > >      int (*grub_setkey_xlat) (int key) = NULL;
> > > >
> > > > Later on in the code, I do the "if (grub_setkey_xlat) blahblahblah"
> > > > stuff as you suggested.
> > > >
> > > > These are the only changes to the 'console.c' source code, exactly as
> > > > what happens in 'grub-core/kern/device.c' with the 'grub_net_open' variable.
> > > >
> > > > Then, in my 'setkey.h' header file, I declare the following:
> > > >
> > > >      extern int (*EXPORT_VAR (grub_setkey_xlat)) (int key);
> > > does conosle include setkey.h?
> > Yes, the 'console.c' source file includes the 'setkey.h' header file.
> > In any case, I took a closer look at the issue, and there is clearly quite a 
> > bit more going on than we are aware of here.
> >
> > First, 'grub-core/Makefile' creates the list of exported kernel symbols based 
> > on the applicable header files.
> > It lists the header files in a variable, appropriately named 
> > 'KERNEL_HEADER_FILES'.
> Well, better name would be KERNEL_HEADER_FILES_WITH_EXPORTED_SYMBOLS.
>
> > The 'setkey.h' header file does not occur in the KERNEL_HEADER_FILES value.
> > Hence, and EXPORT_VAR and EXPORT_FUNC declarations in the 'setkey.h' header 
> > file, will not get picked up by the build process.
> > Any such symbols will, therefore, be considered undefined.
> >
> > To test these assumptions, I decided to manually edit the Makefile (yes, I 
> > know, I shouldn't do that, but it was a quick-and-dirty way to verify if I 
> > understood the issue correctly).
> > Right after the 'configure' step, and before I ran 'make', I manually appended 
> > the path to my 'setkey' file to the KERNEL_HEADER_FILES value.
> > It still didn't work (although the 'setkey.h' file did get picked up when the 
> > list of exported kernel symbols was being constructed).
> > Apparently, the declaration gets ignored because of the 'extern' keyword that 
> > precedes it.
> No, that's very unlikely. Makefile just looks at EXPORT_VAR
> (<alpha-numeric>*) pattern. It ignores anything that comes before or
> after.

Well, as it turns out, you're absolutely right.
My bad - Must have made some stupid error to drive me to that conclusion.
Sorry!

> > So, I created a separate 'console_setkey.h' header file, in which I removed the 
> > 'extern' keyword, and I modified the KERNEL_HEADER_FILES value accordingly.
> > This time, the build process completed without errors.
> > What's more, the 'setkey' command that I had implemented worked perfectly as 
> > well.
> >
> > Since I couldn't immediately find out how I could influence the automatic 
> > generation of the KERNEL_HEADER_FILES value, I then decided to try and add my 
> > EXPORT_VAR declaration directly into the 'console.h' header file.
> > Even though, at first, that seemed to work (no build errors occurred), my 
> > 'setkey' command turned out to be non-functional (i.e., it no longer affected 
> > the keyboard layout).
> > In fact, there were now two instances of the 'grub_setkey_xlat' variable 
> > defined within the kernel, as the following command output illustrates:
> >
> > $ grep 'grub_setkey_xlat' 'grub/grub-core/syminfo.lst'
> > defined linux grub_setkey_xlat
> > defined vga grub_setkey_xlat
> Sure. As you removed extern from declaration, every declaration became
> separate definition, so now you have as many grub_setkey_xlat variables
> defined as there are places that include your header. Unfortunately you
> miss "defined kernel" which is what you actually need.

Well, yes - That's what I also thought.
I was actually quite surprised to see these 'linux' and 'vga' lines; they most 
certainly do not come from a kernel include, I guess.

> > undefined setkey grub_setkey_xlat
> >
> > I assume that the 'linux' instance was the one being used by the console, 
> > while, apparently, by sheer coincidence, my 'setkey' module must have imported 
> > the 'vga' one.
> >
> > SO - WHERE AM I NOW?
> >
> > I have learned that:
> > (1) my 'grub_setkey_xlat' variable (which is to be exported by the console) 
> > will have to be defined in a header file that gets included by (and ONLY by) 
> > 'console.c';
> Again - this is misinterpretation. Many headers with EXPORT_* are
> included in many places. Actually EXPORT_VAR is complete noop unless it
> is compiled with special define.

Well, yes, I understand that EXPORT_VAR and EXPORT_FUNC are noops; they expand 
to the null string.
Their only function (as far as I can tell) is to allow the build process to 
identify exported (kernel) variables and functions.

Further, since by now, I understand that kernel exports should always be 
'defined kernel', I guess it can't hurt to have them included by multiple 
source files after all.
I simply got confused by the 'defined linux' and 'defined vga' lines that I 
saw; I didn't realise that they couldn't have come from the kernel.

Anyway, thanks for the clarifications - they certainly improve my understanding 
of the subject!