[PATCH] dmeventd protocol versioning mechanism

[PATCH] dmeventd protocol versioning mechanism

[Petr Rockai]

Hi,

the attached patch adds rudimentary versioning to the dmevend protocol,
allowing us to detect the (protocol) version of the running dmeventd on
the client side.

Right now this is only used in dmeventd -R.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: dmeventd-protocol-version.diff
Type: text/x-diff
Size: 4492 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20110320/acd84051/attachment.bin>
-------------- next part --------------

Yours,
   Petr

-- 
id' Ash = Ash; id' Dust = Dust; id' _ = undefined

[PATCH] dmeventd protocol versioning mechanism

[Milan Broz]

On 03/20/2011 11:10 AM, Petr Rockai wrote:
+/*
+ * You can (and have to) call this in place of
+ * daemon_talk(fifos, &msg, DM_EVENT_CMD_HELLO, NULL, NULL, 0, 0)
+ * -- this call will parse the version reply from dmeventd, in addition to
+ * above call. It is not safe to call this at any other place in the
+ * protocol.
+ */
+int dm_event_get_version(struct dm_event_fifos *fifos, int *version) {
+ char *p;
+ struct dm_event_daemon_message msg = { 0, 0, NULL };
+
+ if (daemon_talk(fifos, &msg, DM_EVENT_CMD_HELLO, NULL, NULL, 0, 0))
+ return 0;
+ p = msg.data;
+ *version = 0;
+
+ p = strchr(p, ' ') + 1; /* Message ID */
+ p = strchr(p, ' ') + 1; /* HELLO */
+ p = strchr(p, ' '); /* HELLO, once more */


Is this safe? I think it will segfault on malformed reply (no spaces).
strchr(NULL, ...)


Milan

[PATCH] dmeventd protocol versioning mechanism

[Petr Rockai]

Milan Broz <mbroz@redhat.com> writes:

> On 03/20/2011 11:10 AM, Petr Rockai wrote:
> +/*
> + * You can (and have to) call this in place of
> + * daemon_talk(fifos, &msg, DM_EVENT_CMD_HELLO, NULL, NULL, 0, 0)
> + * -- this call will parse the version reply from dmeventd, in addition to
> + * above call. It is not safe to call this at any other place in the
> + * protocol.
> + */
> +int dm_event_get_version(struct dm_event_fifos *fifos, int *version) {
> + char *p;
> + struct dm_event_daemon_message msg = { 0, 0, NULL };
> +
> + if (daemon_talk(fifos, &msg, DM_EVENT_CMD_HELLO, NULL, NULL, 0, 0))
> + return 0;
> + p = msg.data;
> + *version = 0;
> +
> + p = strchr(p, ' ') + 1; /* Message ID */
> + p = strchr(p, ' ') + 1; /* HELLO */
> + p = strchr(p, ' '); /* HELLO, once more */
>
>
> Is this safe? I think it will segfault on malformed reply (no spaces).
> strchr(NULL, ...)

it is not (safe), on the other hand there's code in other places in
libdevmapper-event that assumes correctly formatted messages and will
segfault otherwise. Can be made safe fairly easy though (just adding if
(p)'s) so I'll do that before checking it in.

Yours,
   Petr

-- 
id' Ash = Ash; id' Dust = Dust; id' _ = undefined

[PATCH] dmeventd protocol versioning mechanism

[Milan Broz]

On 03/21/2011 04:32 PM, Petr Rockai wrote:
> it is not (safe), on the other hand there's code in other places in
> libdevmapper-event that assumes correctly formatted messages and will
> segfault otherwise.

Reminds me famous last words: "dmeventd cannot segfault" :-)

We should be more careful at least in new code, it should not hurt much
to reject garbage on input.

Milan