Re: [PATCH] misc: altera-stapl: fix format string vulnerability in OP_PRNT handler

Re: [PATCH] misc: altera-stapl: fix format string vulnerability in OP_PRNT handler

[gregkh]

On Fri, Feb 27, 2026 at 02:30:46PM +0000, Dev Doshi wrote:
> The OP_PRNT opcode handler in the STAPL bytecode interpreter passes
> msg_buff directly as the format string argument to printk():
> 
>     printk(msg_buff, "\n");
> 
> msg_buff is constructed from STAPL bytecode execution through the
> OP_PINT, OP_PCHR, and OP_PSTR opcodes, which append integers,
> characters, and strings from the bytecode's string table. If the
> STAPL/JAM file contains format specifiers (e.g. %p, %x, %n) in its
> string data, these will be interpreted by printk(), potentially
> leaking kernel stack memory or causing undefined behavior.
> 
> The second argument "\n" was clearly intended to append a newline
> after the message, not to serve as a format argument. The programmer
> intended the equivalent of printf("%s\n", msg_buff).
> 
> Fix by using a proper format string with pr_info().
> 
> Assisted-by: GitHub Copilot (Claude claude-4-opus)
> Signed-off-by: Dev Doshi <devdoshi@hotmail.com>
> 
> ---
>  drivers/misc/altera-stapl/altera.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/altera-stapl/altera.c b/drivers/misc/altera-stapl/altera.c
> index 4fa6c9062..b9a5a3ea7 100644
> --- a/drivers/misc/altera-stapl/altera.c
> +++ b/drivers/misc/altera-stapl/altera.c
> @@ -700,7 +700,7 @@ static int altera_execute(struct altera_state *astate,
>                 case OP_PRNT:
>                         /* PRINT finish */
>                         if (debug)
> -                               printk(msg_buff, "\n");
> +                               pr_info("%s\n", msg_buff);

Close, but not quite :)

pr_info() has a different output format than printk() does.  Or it can,
depending on the calling file.  Do you have this hardware to test that
this really looks the same?  And this data is coming from hardware, not
userspace, right?

I think that should be written as:
		case OP_PRNG:
			/* PRINT finish */
			dprintk("%s\n", msg_buff);

making it one less line overall, and "fixing" the potential issue at the
same time, a win for everyone :)

This whole file is really crazy, and given the amount of noise is is
printing out to the kernel log, I can't imagine anyone is actually using
it in this manner as it's just a lot of fpga junk.

thanks,

greg k-h