dwfl: fix potential overflow when reporting on kernel modules

Message ID 20211118194449.1931428-1-maennich@google.com
State Committed
Headers
Series dwfl: fix potential overflow when reporting on kernel modules |

Commit Message

Matthias Männich Nov. 18, 2021, 7:44 p.m. UTC
  dwfl_linux_kernel_report_modules_ has an outstanding ancient bug when
reading kernel module information from a modules list file. The target
buffer for the module name was sized too small to hold potential values.
Fix that by increasing the value to account for the null termination.

In practice, this unlikely ever happened, but it now got diagnosed by
LLVM as part of a stricter -Wfortify-source implementation [1]:

libdwfl/linux-kernel-modules.c:1019:7: error: 'sscanf' may overflow; destination buffer in argument 3 has size 128, but the corresponding specifier may require size 129 [-Werror,-Wfortify-source]
                    modname, &modsz, &modaddr) == 3)

[1] https://github.com/llvm/llvm-project/commit/2db66f8d48beeea835cb9a6940e25bc04ab5d941

Suggested-by: Paul Pluzhnikov <ppluzhnikov@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
Change-Id: I97b4fcb536273e5ccc4e37b0b9f0f8ffb7487909
---
 libdwfl/linux-kernel-modules.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Dmitry V. Levin Nov. 18, 2021, 10:04 p.m. UTC | #1
On Thu, Nov 18, 2021 at 07:44:50PM +0000, Matthias Maennich via Elfutils-devel wrote:
> dwfl_linux_kernel_report_modules_ has an outstanding ancient bug when
> reading kernel module information from a modules list file. The target
> buffer for the module name was sized too small to hold potential values.
> Fix that by increasing the value to account for the null termination.
> 
> In practice, this unlikely ever happened, but it now got diagnosed by
> LLVM as part of a stricter -Wfortify-source implementation [1]:
> 
> libdwfl/linux-kernel-modules.c:1019:7: error: 'sscanf' may overflow; destination buffer in argument 3 has size 128, but the corresponding specifier may require size 129 [-Werror,-Wfortify-source]
>                     modname, &modsz, &modaddr) == 3)
> 
> [1] https://github.com/llvm/llvm-project/commit/2db66f8d48beeea835cb9a6940e25bc04ab5d941
> 
> Suggested-by: Paul Pluzhnikov <ppluzhnikov@google.com>
> Signed-off-by: Matthias Maennich <maennich@google.com>
> Change-Id: I97b4fcb536273e5ccc4e37b0b9f0f8ffb7487909
> ---
>  libdwfl/linux-kernel-modules.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libdwfl/linux-kernel-modules.c b/libdwfl/linux-kernel-modules.c
> index c0f8dfa4aef2..58c0c417a597 100644
> --- a/libdwfl/linux-kernel-modules.c
> +++ b/libdwfl/linux-kernel-modules.c
> @@ -1008,7 +1008,7 @@ dwfl_linux_kernel_report_modules (Dwfl *dwfl)
>    int result = 0;
>    Dwarf_Addr modaddr;
>    unsigned long int modsz;
> -  char modname[128];
> +  char modname[128+1];
>    char *line = NULL;
>    size_t linesz = 0;
>    /* We can't just use fscanf here because it's not easy to distinguish \n

LGTM, thanks.
  
Mark Wielaard Nov. 20, 2021, 12:26 a.m. UTC | #2
Hi,

On Fri, Nov 19, 2021 at 01:04:39AM +0300, Dmitry V. Levin wrote:
> On Thu, Nov 18, 2021 at 07:44:50PM +0000, Matthias Maennich via Elfutils-devel wrote:
> > dwfl_linux_kernel_report_modules_ has an outstanding ancient bug when
> > reading kernel module information from a modules list file. The target
> > buffer for the module name was sized too small to hold potential values.
> > Fix that by increasing the value to account for the null termination.
> > 
> > In practice, this unlikely ever happened, but it now got diagnosed by
> > LLVM as part of a stricter -Wfortify-source implementation [1]:
> > 
> > libdwfl/linux-kernel-modules.c:1019:7: error: 'sscanf' may overflow; destination buffer in argument 3 has size 128, but the corresponding specifier may require size 129 [-Werror,-Wfortify-source]
> >                     modname, &modsz, &modaddr) == 3)
> > 
> > [1] https://github.com/llvm/llvm-project/commit/2db66f8d48beeea835cb9a6940e25bc04ab5d941
> > 
> > Suggested-by: Paul Pluzhnikov <ppluzhnikov@google.com>
> > Signed-off-by: Matthias Maennich <maennich@google.com>
> > Change-Id: I97b4fcb536273e5ccc4e37b0b9f0f8ffb7487909
> > ---
> >  libdwfl/linux-kernel-modules.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libdwfl/linux-kernel-modules.c b/libdwfl/linux-kernel-modules.c
> > index c0f8dfa4aef2..58c0c417a597 100644
> > --- a/libdwfl/linux-kernel-modules.c
> > +++ b/libdwfl/linux-kernel-modules.c
> > @@ -1008,7 +1008,7 @@ dwfl_linux_kernel_report_modules (Dwfl *dwfl)
> >    int result = 0;
> >    Dwarf_Addr modaddr;
> >    unsigned long int modsz;
> > -  char modname[128];
> > +  char modname[128+1];
> >    char *line = NULL;
> >    size_t linesz = 0;
> >    /* We can't just use fscanf here because it's not easy to distinguish \n
> 
> LGTM, thanks.

Agreed. Added a ChangeLog entry and pushed.

Thanks,

Mark
  

Patch

diff --git a/libdwfl/linux-kernel-modules.c b/libdwfl/linux-kernel-modules.c
index c0f8dfa4aef2..58c0c417a597 100644
--- a/libdwfl/linux-kernel-modules.c
+++ b/libdwfl/linux-kernel-modules.c
@@ -1008,7 +1008,7 @@  dwfl_linux_kernel_report_modules (Dwfl *dwfl)
   int result = 0;
   Dwarf_Addr modaddr;
   unsigned long int modsz;
-  char modname[128];
+  char modname[128+1];
   char *line = NULL;
   size_t linesz = 0;
   /* We can't just use fscanf here because it's not easy to distinguish \n