linux-kernel - Re: [PATCH 06/14] DRBD: userspace

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: <e2e108260904120923q639d036drb04b55b1e653ebf1@mail.gmail.com>
Date:	Sun, 12 Apr 2009 18:23:43 +0200
From:	Bart Van Assche <bart.vanassche@...il.com>
To:	Philipp Reisner <philipp.reisner@...bit.com>
Cc:	linux-kernel@...r.kernel.org, Jens Axboe <jens.axboe@...cle.com>,
	Greg KH <gregkh@...e.de>, Neil Brown <neilb@...e.de>,
	James Bottomley <James.Bottomley@...senpartnership.com>,
	Andi Kleen <andi@...stfloor.org>,
	Sam Ravnborg <sam@...nborg.org>, Dave Jones <davej@...hat.com>,
	Nikanth Karthikesan <knikanth@...e.de>,
	Lars Marowsky-Bree <lmb@...e.de>,
	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>,
	Lars Ellenberg <lars.ellenberg@...bit.com>
Subject: Re: [PATCH 06/14] DRBD: userspace_interface

On Fri, Apr 10, 2009 at 2:12 PM, Philipp Reisner
<philipp.reisner@...bit.com> wrote:
> diff -uNrp linux-2.6.30-rc1/include/linux/drbd.h linux-2.6.30-rc1-drbd/include/linux/drbd.h
> --- linux-2.6.30-rc1/include/linux/drbd.h       1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.30-rc1-drbd/include/linux/drbd.h  2009-03-26 15:53:46.520275000 +0100

...

> +#include <linux/drbd_config.h>

By including drbd_config.h in drbd.h all definitions in the former
header file become visible in user space. Several definitions in
drbd_config.h only make sense inside the kernel. Either the above
#include directive should be removed or drbd_config.h should be
cleaned up.

...

> +/* Altough the Linux source code makes a difference between
> +   generic endiness and the bitfields' endianess, there is no
> +   architecture as of Linux-2.6.24-rc4 where the bitfileds' endianess
> +   does not match the generic endianess. */

I assume this should have been four times "endianness", just like below ?

> +
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> +#define __LITTLE_ENDIAN_BITFIELD
> +#elif __BYTE_ORDER == __BIG_ENDIAN
> +#define __BIG_ENDIAN_BITFIELD
> +#else
> +# error "sorry, weird endianness on this box"
> +#endif

...

> +/* KEEP the order, do not delete or insert!
> + * Or change the API_VERSION, too. */
> +enum ret_codes {
> +       RetCodeBase = 100,
> +       NoError,         /* 101 ... */
> +       LAAlreadyInUse,

How will backwards compatibility for return codes be ensured ? The
comment before the enum probably has to be changed to "KEEP the order,
do not delete or insert!" only ?

...

> +union drbd_state_t {
> +/* According to gcc's docs is the ...
> + * The order of allocation of bit-fields within a unit (C90 6.5.2.1, C99 6.7.2.1).
> + * Determined by ABI.
> + * pointed out by Maxim Uvarov q<muvarov@...mvista.com>
> + * even though we transmit as "cpu_to_be32(state)",
> + * the offsets of the bitfields still need to be swapped
> + * on different endianess.
> + */

The above comment really looks strange. Is it still up to date ?

...

> +/* from drbd_strings.c */
> +extern const char *conns_to_name(enum drbd_conns);
> +extern const char *roles_to_name(enum drbd_role);
> +extern const char *disks_to_name(enum drbd_disk_state);
> +extern const char *set_st_err_name(enum set_st_err);

Should declarations of kernel functions really be present in a user
space interface header ?

Bart.
--
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/