linux-kernel - Re: [PATCH 068/118] drbd: conn_printk() a dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1314296185.15882.75.camel@Joe-Laptop>
Date:	Thu, 25 Aug 2011 11:16:25 -0700
From:	Joe Perches <joe@...ches.com>
To:	Philipp Reisner <philipp.reisner@...bit.com>
Cc:	linux-kernel@...r.kernel.org, Jens Axboe <axboe@...nel.dk>,
	drbd-dev@...ts.linbit.com
Subject: Re: [PATCH 068/118] drbd: conn_printk() a dev_printk() alike for
 drbd's connections

On Thu, 2011-08-25 at 17:08 +0200, Philipp Reisner wrote:
> Signed-off-by: Philipp Reisner <philipp.reisner@...bit.com>
> Signed-off-by: Lars Ellenberg <lars.ellenberg@...bit.com>
> ---
>  drivers/block/drbd/drbd_int.h  |    9 +++++++++
>  drivers/block/drbd/drbd_main.c |   16 +++++++++++++++-
>  2 files changed, 24 insertions(+), 1 deletions(-)

Couple of issues with this patch.

> diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h
[]
> @@ -102,6 +102,15 @@ struct drbd_tconn;
[]
> +extern void conn_printk(const char *level, struct drbd_tconn *tconn, const char *fmt, ...);

This should be 

extern __attribute__((format (printf, 3, 4)))
void conn_printk(const char *level, struct drbd_tconn *tconn, const char  *fmt, ...);

to have gcc validate the printf format and arguments.

> diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
[]
> @@ -170,6 +170,18 @@ int _get_ldev_if_state(struct drbd_conf *mdev, enum drbd_disk_state mins)
>  
>  #endif
>  
> +/* printk functions for connections
> + */
> +void conn_printk(const char *level, struct drbd_tconn *tconn, const char *fmt, ...)
> +{
> +	va_list args;
> +
> +	printk("%sd-con %s: ", level, tconn->name);
> +	va_start(args, fmt);
> +	vprintk(fmt, args);
> +	va_end(args);

And using printk then vprintk is susceptible
to another thread interleaving a different
message between the printk and the vprintk.

Using struct va_format and %pV is better
because no message interleaving is possible.

ie:

void conn_printk(const char *level, struct drbd_tconn *tconn, const char *fmt, ...)
{
	struct va_format vaf;
	va_list args;

	va_start(args, fmt);

	vaf.fmt = fmt;
	vaf.va = &args;

	printk("%sd-con %s: %pV", level, tconn->name, &vaf);

	va_end(args);
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/