Normally exported symbol's crc is stored as absolute (SHN_ABS)
value of special named symbol __crc_<symbol name>.
When the kernel and modules are built with the config option
CONFIG_MODULE_REL_CRCS, all the CRCs are put in a special section
and the __crc_<symbol name> symbols values are offsets in the
section. See patch description of the commit:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=56067812d5b0e737ac2063e94a50f76b810d6ca3
Add kmod support of this configuration.
Signed-off-by: Yauheni Kaliuta <[email protected]>
---
libkmod/libkmod-elf.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/libkmod/libkmod-elf.c b/libkmod/libkmod-elf.c
index 90da89aebbaf..ef4a8a3142a1 100644
--- a/libkmod/libkmod-elf.c
+++ b/libkmod/libkmod-elf.c
@@ -747,6 +747,31 @@ static inline uint8_t kmod_symbol_bind_from_elf(uint8_t elf_value)
}
}
+static uint64_t kmod_elf_resolve_crc(const struct kmod_elf *elf, uint64_t crc, uint16_t shndx)
+{
+ int err;
+ uint64_t off, size;
+ uint32_t nameoff;
+
+ if (shndx == SHN_ABS || shndx == SHN_UNDEF)
+ return crc;
+
+ err = elf_get_section_info(elf, shndx, &off, &size, &nameoff);
+ if (err < 0) {
+ ELFDBG("Cound not find section index %"PRIu16" for crc", shndx);
+ return (uint64_t)-1;
+ }
+
+ if (crc > (size - sizeof(uint32_t))) {
+ ELFDBG("CRC offset %"PRIu64" is too big, section %"PRIu16" size is %"PRIu64"\n",
+ crc, shndx, size);
+ return (uint64_t)-1;
+ }
+
+ crc = elf_get_uint(elf, off + crc, sizeof(uint32_t));
+ return crc;
+}
+
/* array will be allocated with strings in a single malloc, just free *array */
int kmod_elf_get_symbols(const struct kmod_elf *elf, struct kmod_modversion **array)
{
@@ -830,6 +855,7 @@ int kmod_elf_get_symbols(const struct kmod_elf *elf, struct kmod_modversion **ar
uint32_t name_off;
uint64_t crc;
uint8_t info, bind;
+ uint16_t shndx;
#define READV(field) \
elf_get_uint(elf, sym_off + offsetof(typeof(*s), field),\
@@ -839,11 +865,13 @@ int kmod_elf_get_symbols(const struct kmod_elf *elf, struct kmod_modversion **ar
name_off = READV(st_name);
crc = READV(st_value);
info = READV(st_info);
+ shndx = READV(st_shndx);
} else {
Elf64_Sym *s;
name_off = READV(st_name);
crc = READV(st_value);
info = READV(st_info);
+ shndx = READV(st_shndx);
}
#undef READV
name = elf_get_mem(elf, str_off + name_off);
@@ -856,7 +884,7 @@ int kmod_elf_get_symbols(const struct kmod_elf *elf, struct kmod_modversion **ar
else
bind = ELF64_ST_BIND(info);
- a[count].crc = crc;
+ a[count].crc = kmod_elf_resolve_crc(elf, crc, shndx);
a[count].bind = kmod_symbol_bind_from_elf(bind);
a[count].symbol = itr;
slen = strlen(name);
--
2.14.0.rc0
On Wed, Jul 19, 2017 at 7:56 AM, Yauheni Kaliuta
<[email protected]> wrote:
> Normally exported symbol's crc is stored as absolute (SHN_ABS)
> value of special named symbol __crc_<symbol name>.
>
> When the kernel and modules are built with the config option
> CONFIG_MODULE_REL_CRCS, all the CRCs are put in a special section
> and the __crc_<symbol name> symbols values are offsets in the
> section. See patch description of the commit:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=56067812d5b0e737ac2063e94a50f76b810d6ca3
>
> Add kmod support of this configuration.
>
> Signed-off-by: Yauheni Kaliuta <[email protected]>
> ---
> libkmod/libkmod-elf.c | 30 +++++++++++++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/libkmod/libkmod-elf.c b/libkmod/libkmod-elf.c
> index 90da89aebbaf..ef4a8a3142a1 100644
> --- a/libkmod/libkmod-elf.c
> +++ b/libkmod/libkmod-elf.c
> @@ -747,6 +747,31 @@ static inline uint8_t kmod_symbol_bind_from_elf(uint8_t elf_value)
> }
> }
>
> +static uint64_t kmod_elf_resolve_crc(const struct kmod_elf *elf, uint64_t crc, uint16_t shndx)
> +{
> + int err;
> + uint64_t off, size;
> + uint32_t nameoff;
> +
> + if (shndx == SHN_ABS || shndx == SHN_UNDEF)
> + return crc;
> +
> + err = elf_get_section_info(elf, shndx, &off, &size, &nameoff);
> + if (err < 0) {
> + ELFDBG("Cound not find section index %"PRIu16" for crc", shndx);
> + return (uint64_t)-1;
> + }
> +
> + if (crc > (size - sizeof(uint32_t))) {
> + ELFDBG("CRC offset %"PRIu64" is too big, section %"PRIu16" size is %"PRIu64"\n",
> + crc, shndx, size);
> + return (uint64_t)-1;
> + }
> +
> + crc = elf_get_uint(elf, off + crc, sizeof(uint32_t));
> + return crc;
> +}
> +
> /* array will be allocated with strings in a single malloc, just free *array */
> int kmod_elf_get_symbols(const struct kmod_elf *elf, struct kmod_modversion **array)
> {
> @@ -830,6 +855,7 @@ int kmod_elf_get_symbols(const struct kmod_elf *elf, struct kmod_modversion **ar
> uint32_t name_off;
> uint64_t crc;
> uint8_t info, bind;
> + uint16_t shndx;
>
> #define READV(field) \
> elf_get_uint(elf, sym_off + offsetof(typeof(*s), field),\
> @@ -839,11 +865,13 @@ int kmod_elf_get_symbols(const struct kmod_elf *elf, struct kmod_modversion **ar
> name_off = READV(st_name);
> crc = READV(st_value);
> info = READV(st_info);
> + shndx = READV(st_shndx);
> } else {
> Elf64_Sym *s;
> name_off = READV(st_name);
> crc = READV(st_value);
> info = READV(st_info);
> + shndx = READV(st_shndx);
> }
> #undef READV
> name = elf_get_mem(elf, str_off + name_off);
> @@ -856,7 +884,7 @@ int kmod_elf_get_symbols(const struct kmod_elf *elf, struct kmod_modversion **ar
> else
> bind = ELF64_ST_BIND(info);
>
> - a[count].crc = crc;
> + a[count].crc = kmod_elf_resolve_crc(elf, crc, shndx);
> a[count].bind = kmod_symbol_bind_from_elf(bind);
> a[count].symbol = itr;
> slen = strlen(name);
> --
LGTM. I'll give this a try and apply. Do you think it's possible to
build an out-of-tree module with this option so we can add one to the
testsuite?
Lucas De Marchi
Hi, Lucas!
>>>>> On Wed, 19 Jul 2017 10:51:44 -0700, Lucas De Marchi wrote:
> On Wed, Jul 19, 2017 at 7:56 AM, Yauheni Kaliuta
> <[email protected]> wrote:
>> Normally exported symbol's crc is stored as absolute (SHN_ABS)
>> value of special named symbol __crc_<symbol name>.
>>
>> When the kernel and modules are built with the config option
>> CONFIG_MODULE_REL_CRCS, all the CRCs are put in a special section
>> and the __crc_<symbol name> symbols values are offsets in the
>> section. See patch description of the commit:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=56067812d5b0e737ac2063e94a50f76b810d6ca3
>>
>> Add kmod support of this configuration.
[...]
> LGTM. I'll give this a try and apply.
Thanks! I would also like to get some feedback from the kernel feature
author, Ard Biesheuvel, that this does not miss other usecases.
> Do you think it's possible to build an out-of-tree module with this
> option so we can add one to the testsuite?
I'll check what I can do.
The problem is seen if you run depmod with -e -E, which I guess is not used
too often.
--
WBR,
Yauheni Kaliuta
On 19 July 2017 at 20:04, Yauheni Kaliuta <[email protected]> wrote:
> Hi, Lucas!
>
>>>>>> On Wed, 19 Jul 2017 10:51:44 -0700, Lucas De Marchi wrote:
> > On Wed, Jul 19, 2017 at 7:56 AM, Yauheni Kaliuta
> > <[email protected]> wrote:
> >> Normally exported symbol's crc is stored as absolute (SHN_ABS)
> >> value of special named symbol __crc_<symbol name>.
> >>
> >> When the kernel and modules are built with the config option
> >> CONFIG_MODULE_REL_CRCS, all the CRCs are put in a special section
> >> and the __crc_<symbol name> symbols values are offsets in the
> >> section. See patch description of the commit:
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=56067812d5b0e737ac2063e94a50f76b810d6ca3
> >>
> >> Add kmod support of this configuration.
>
> [...]
>
> > LGTM. I'll give this a try and apply.
>
> Thanks! I would also like to get some feedback from the kernel feature
> author, Ard Biesheuvel, that this does not miss other usecases.
>
Hi all,
The patch looks correct to me. My only comment is that
kmod_elf_resolve_crc() should probably return a uint32_t instead,
given the size of a CRC32.
In any case,
Reviewed-by: Ard Biesheuvel <[email protected]>
Thanks,
Ard.
> > Do you think it's possible to build an out-of-tree module with this
> > option so we can add one to the testsuite?
>
> I'll check what I can do.
>
> The problem is seen if you run depmod with -e -E, which I guess is not used
> too often.
>
> --
> WBR,
> Yauheni Kaliuta
Hi, Ard!
>>>>> On Wed, 19 Jul 2017 20:10:17 +0100, Ard Biesheuvel wrote:
> On 19 July 2017 at 20:04, Yauheni Kaliuta <[email protected]> wrote:
>> Hi, Lucas!
>>
>>>>>>> On Wed, 19 Jul 2017 10:51:44 -0700, Lucas De Marchi wrote:
>> > On Wed, Jul 19, 2017 at 7:56 AM, Yauheni Kaliuta
>> > <[email protected]> wrote:
>> >> Normally exported symbol's crc is stored as absolute (SHN_ABS)
>> >> value of special named symbol __crc_<symbol name>.
>> >>
>> >> When the kernel and modules are built with the config option
>> >> CONFIG_MODULE_REL_CRCS, all the CRCs are put in a special section
>> >> and the __crc_<symbol name> symbols values are offsets in the
>> >> section. See patch description of the commit:
>> >>
>> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=56067812d5b0e737ac2063e94a50f76b810d6ca3
>> >>
>> >> Add kmod support of this configuration.
>>
>> [...]
>>
>> > LGTM. I'll give this a try and apply.
>>
>> Thanks! I would also like to get some feedback from the kernel feature
>> author, Ard Biesheuvel, that this does not miss other usecases.
>>
> The patch looks correct to me. My only comment is that
> kmod_elf_resolve_crc() should probably return a uint32_t instead,
> given the size of a CRC32.
Well, it's a specific of the internal libkmod implementation. Both
struct kmod_modversion::crc and return value of elf_get_uint()
are uint64_t, but in the patch elf_get_uint() reads correct
sizeof(uint32_t) value from the ELF.
> In any case,
> Reviewed-by: Ard Biesheuvel <[email protected]>
Thanks a lot!
--
WBR,
Yauheni Kaliuta