stratis-storage/project

dbus binding doesn't detect values with incorrect types in arrays, it elides them

trgill opened this issue · 6 comments

trgill commented

For example, the following uses an incorrect a(sss), but the call gets through...

busctl --user call org.storage.stratis1 /org/storage/stratis1/1 org.storage.stratis1.pool CreateFilesystems a(sss) 3 volume1 /dev/mount/point 10 volume2 /dev/mount/point2 11 volume3 /dev/mount/point3 12
a(oqs)qs 00 "Ok"

It doesn't seem to be a big deal - the filesystems array is 0 length.

mulkieran commented

I'm guessing that busctl never sends the actual signature over the dbus, just the values, packaged according to the signature information you gave it. I bet stratisd would have an issue if one of those numbers were negative, or otherwise didn't fit the uint64 requirements.

agrover commented

OK, what happens when one of the numbers is negative?

mulkieran commented

Whoops, pretty funky.

[mulhern@dhcp-25-209 stratisd]$ busctl --user call org.storage.stratis1 /org/storage/stratis1/1 org.storage.stratis1.pool CreateFilesystems "a(sss)" 1 volume1 /dev/mount/point str
a(oqs)qs 00 "Ok"

I can't get the same effect unless it's a value in array. Might be a dbus-rs problem, since we seem to be using the correct types.

mulkieran commented

Known dbus-rs bug, looks like: diwic/dbus-rs#62. There may be a work around.

mulkieran commented

All our changes to D-Bus layer haven't removed this problem, alas. What's slightly wierder is that:

[root@localhost stratisd]# busctl --system call org.storage.stratis1 /org/storage/stratis1 org.storage.stratis1.Manager CreatePool "s(bq)ban" name1 0 0 0 1 2

succeeds, but doesn't return the object path for the blockdev, which was passed as a numeric value.

(oao)qs "/org/storage/stratis1/3" 0 0 "Ok"

There's a real possibility that dbus-rs is just skipping the value that was incorrectly packaged. After all, this

[root@localhost stratisd]# busctl --system call org.storage.stratis1 /org/storage/stratis1 org.storage.stratis1.Manager CreatePool "s(bq)bas" name2 0 0 0 1 2
(oao)qs "/org/storage/stratis1/4" 1 "/org/storage/stratis1/5" 0 "Ok"

works. Not clear what to do about this, though.

mulkieran commented

We will document that this is an allowed bug in the README.