FileDescriptors don't actually get sent about dbus-java HOT 49 CLOSED

file descriptors are really hard ...

The current implementation just sends the number of the file descriptor so the calling client can create a new FileDescriptor object using the given number to access the same file.

Adding 'NEGOTIATE_UNIX_FD' and 'AGREE_UNIX_FD' is not a problem.
My problem is to understand what "index into an out-of-band array of file descriptors" is supposed to mean (why an array of file descriptors?).
The documentation is not very specific on how to deal with file descriptors.

Maybe my knowledge of the english language is not good enough.
Maybe the documentation is just crap at that point.

If you can help provide patches, hints or anything useful, please let me know

from dbus-java.

The problem with sending just the number is that there's no way to take that number and turn it back into a file, since the filedescriptors are unique per-process.

I can take a look at this later this week, but some things that I have found:
C#/Mono code that adds file descriptors: mono/dbus-sharp#62
It looks like jnr-unixsocket doesn't support this at the moment, as what needs to happen is that sendmsg needs to be used instead of sendto(which is what is being used at the moment).

See also this StackOverflow post on doing this in C: https://stackoverflow.com/questions/28003921/sending-file-descriptor-by-linux-socket

from dbus-java.

After some speluking through the libdbus code, I think that I have found the method which actually does the writing out.
So it seems that when the specification says

Unsigned 32-bit integer representing an index into an out-of-band array of file descriptors, transferred via some platform-specific mechanism

It is talking about setting certain fields in a struct cmsghdr, which is part of struct msghdr. This explains why the actual number of the filedescriptor needs to be an index into this array, and looking at dbus_message_iter_append_basic we see that the filedescriptors are basically copied over to their own separate array which is then sent out as part of the sendmsg call previously seen.

from dbus-java.

Question: is it required to continue to use jnr-unixsocket? I came across this other project just now that already has support for FileDescriptors over a Unix socket. This would swap out some dependencies, but I'm not sure if that would be acceptable or not.

from dbus-java.

The reason for using jnr-unixsocket is that they do not require any native-compiled wrapper library.

As far as I can see junixsocket uses a wrapper as well.
This is like the old libmatthew which also requird native library to be available.

Because linux (and therefore DBus) is running on many platforms with many different architectures it was always a pain providing native libraries for these platforms.
Different platform versions (e.g. ARMv6/v7) also were annoying.
In there cases the developer of the application has to figure out how to compile the native library and how to use it in combination with dbus-java.

I think this is inconvienent, so that's the main reason why using jnr-unixsockets and that's why I don't want to switch back to another library doing the same stuff I want to avoid.

But maybe we can provide a patch to the jnr-unixsocket maintainer to support the changes we need.
I did this a while ago (litte patch to support the PASS_CRED feature) when implementing jnr-unixsocket in dbus-java.

I'll try to take a look into the implementation of this stuff in junixsocket next week, maybe I can write that patch.

Alternativly we could change dbus-java to support different 'transport' libraries, so if you need Filedescriptors (rare case in my opinion), you have to add the dbus-java-junixsocket transport, otherwise you can use the dbus-java-jnr-unixsocket transport ...
But lets wait until we are really sure that this kind of feature is really required.

from dbus-java.

For using JNR, I've started on some work to see if it is possible. This is difficult because there's not much documentation on how to use JNR, JNA is much more prevalent. Branch is here: https://github.com/rm5248/jnr-unixsocket/tree/rm-testing

The problem that I have just run into when attempting to write a test for this is that there are certain macros that must be used in order for the cmsghdr to be set properly. According to the man page though:

Ancillary data is a sequence of struct cmsghdr structures with appended data. This sequence should only be accessed using the macros described in this manual page and never directly.

If that is true, then it seems likely that there's no way to do this from pure Java and we may need to make some JNI shim library to handle it.

from dbus-java.

Maybe we get the job done, if we include/use jnr-posix. They already added the missing sendmsg/recvmsg and also some of the macros you mentioned.

from dbus-java.

I pushed my changes to filedescriptors branch.
I did not implement the stuff required for sendmsg/recvmsg, but already included jnr-posix as dependency.

I tried to add the required handshake-stuff so DBus knows that file descriptor passing is possible (NEGOTIATE_UNIX_FD).

from dbus-java.

I have at least a partial implementation at the moment. I think the basic code is correct, but it's not talking with the bus at the moment. My branch is here: https://github.com/rm5248/dbus-java/tree/filedescriptor-passing

The problem that I have at the moment is that the iovec is null(check FDMessageWriter):

19:22:32.236 [DBus Sender Thread-1] TRACE org.freedesktop.dbus.FDMessageWriter - msghdr to send: msghdr {
  msg_name=null,
  msg_namelen=0,
  msg_iov=[
    iovec {
      iov_base=null,
      iov_len=128,
    }
  ],
  msg_control=[
  ],
  msg_controllen=0
  msg_iovlen=1,
  msg_flags=0,
}

Since no data is ever written, we can never talk with the bus(the Hello message doesn't go out). I've tracked it down to what JNR-posix is doing with Pointer.wrap with a ByteBuffer. Just before that there's this line:

logger.debug( "pointer is {}", 
                Pointer.wrap( jnr.ffi.Runtime.getSystemRuntime(), dataSend ) );

Which is giving me a null pointer:

19:22:32.234 [DBus Sender Thread-1] DEBUG org.freedesktop.dbus.FDMessageWriter - pointer is jnr.ffi.provider.jffi.ByteBufferMemoryIO[address=0x0 size=128]

So this looks like a bug in JNR, but it seems weird that it wouldn't work correctly. Am I missing something?

from dbus-java.

I'm still struggling with a proper test setup. How did you do the testing?

I noticed something in FDMessageWriter class.

On line 63 you use msghdr.allocateControl with the size of the list (e.g. 1 if only 1 FD is in the list).
As far as I understand the datalength is needed here, and I don't think that one FD is only one byte in length.
When comparing it to a test case of jnr-posix, I see that they are using a length 4 (4 bytes) which seems more appropriate to me (see here: jnr-posix IOTest).

Also there is already a constant for SOL_SOCKET in jnr-posix, so you may switch to that one instead of using your own (jnr.constants.platform.SocketLevel.SOL_SOCKET.intValue())

from dbus-java.

At the moment, I'm just trying to run the currently-existing tests, and those are failing because it can't connect to the bus correctly. The repo that I linked before also has an FDTX and FDRX classes; currently I'm focused only on sending, receiving requires more work still.

That IOTest from JNR looks useful; there's not much information on how to use JNR so I've been going around somewhat blindly.

from dbus-java.

I updated my branch with the fixes required to send messages at least, and all of the tests except for the file descriptor are working at the moment. I started work on the receiving end, but that requires more work still on me understanding what the spec says and what the code is doing with the InputStream in order to read the data.

from dbus-java.

Nice! Keep up the great work!
I'll try to help if I find some spare time to do so (and if I can finally understand the specs ...)

from dbus-java.

Okay, so the good news at this point: sendmsg and recvmsg are both implemented for all the current tests. I've run into a few issues at this point though.

• There's a bug in jnr-posix(fixed on my branch here) that caused an infinite loop.
• The tests(not including the filedescriptor test) occasionally fail with a ConcurrentModificationException in the DBusConnectionTest(test_busnames_should_be_auto_released_on_close_of_non_shared_connection)
• Sometimes it also fails with the TestEmptyCollections test.
• The FileDescriptor test is failing to extract properly; the toString is failing

Some of these seem like race conditions, but I'm not sure why they wouldn't have come up before.

At this point, it seems like I need to update Message.java, as simply adding a new header(UNIX_FDS) doesn't automatically add this to the marshalled message. This may be the cause of the toString failure on the Message object.

If you are able to take a look, that would be helpful.

from dbus-java.

I'll take a look at that concurrent modification exception.
There are some test cases which were not stable, it's about time to get this fixed.

About your toString issue:
I'm not sure if the code is correct yet.

The exception is caused by trying to read an arraylist without content (e.g. filedescriptors.get(0) when list is empty).
When 'fixing' this issue by checking the list of being empty first, the unit test will fail because no file descriptor is returned but null.

So my guess is, that whatever should be in the filedescriptor list is never written to it.
Did not have the time to debug this, yet.

from dbus-java.

I messed around with this a bit more, and I just got the file descriptors to at least be returned correctly(I think). This shows up in dbus-monitor:

method call time=1575223072.038383 sender=:1.3140 -> destination=foo.bar.TestFileDescriptor serial=5 path=/TestFileDescriptor; interface=foo.bar.TestFileDescriptor; member=getFileDescriptor
method return time=1575223072.044919 sender=:1.3140 -> destination=:1.3140 serial=6 reply_serial=5
   file descriptor
         inode: 5739061
         type: fifo

The fix for this is pretty ugly though, since the header needs to be written before any of the data. To fix it properly however requires a likely significant rework of the Message class.

However, reading them doesn't work correctly at the moment; I'm unsure as to what exactly I need to do in order to read it correctly. Possibly because the reading is doing a MSG_PEEK before reading the data the control data is discarded?

from dbus-java.

The fix for this is pretty ugly though, since the header needs to be written before any of the data.

Most of the code of this project is ugly anyway, so never mind.

My understanding of MSG_PEEK is, that it will look ahead in the stream for data and do not consume it, so read calls can pick it up later.
That is also what the man page says:
MSG_PEEK This flag causes the receive operation to return data from the beginning of the receive queue without removing that data from the queue. Thus, a subsequent receive call will return the same data.

see here

Maybe you have to read only the 12 bytes from the header and not the additional 4 bytes for the array index as this is already 'payload' and not header. So maybe those 4 bytes are missing later on (just guessing ...)

from dbus-java.

I also did some testing without any usable results.
For me it looks like if the control data put on the MsgHdr is getting lost or not read correctly.

If I change the size in allocateControl method to the size I would expect, I get the 'control data has been truncated' flag on the message. If it is larger sometimes the test gets stucks, sometimes I get control data , but not the data I placed into it (e.g. level == 2 instead of 1)...

from dbus-java.

So I think that the sending of the file descriptors is still not entirely correct. I do get an apparently valid file descriptor on my C++ side, however I cam unable to write to the file descriptor. I need to update my java/c++ examples to make this much easier to test; there's a lot of hand-editing of several files that I'm doing for testing and it's hard to keep it all straight in my mind.

On a somewhat related note, I'm doing this with Java 11, and I'm getting a warning about disallowed access for java.io.FileDescriptor(since we are constructing it via reflection). This should probably be removed once we have this finalized, since it will fail in a future version.

from dbus-java.

So I think that the sending of the file descriptors is still not entirely correct.
That was what I'm also thinking about.

Which size should sendMsg return when writing a message? Just the "payload" or should it also include the size of the appended control data?
I always see that 52 are written, but these 52 bytes are only the payload, the control data would require additional 24 byte.
Also the reading part always receives 68 bytes while only 52 were send..?

On a somewhat related note, I'm doing this with Java 11, and I'm getting a warning about disallowed access for java.io.FileDescriptor(since we are constructing it via reflection). This should probably be removed once we have this finalized, since it will fail in a future version

I'm also using Java 11 for development and saw this issue already.
I don't like the reflection stuff anyway, but are there any suitable alternatives?

If you have any usable test setup, please share it so I may help as well :)

from dbus-java.

Okay, I've updated my test setup here: https://github.com/rm5248/dbus-java-filedescriptor

To use:

1. Run the 'ReturnFD' java program in the 'test' package.
2. Run the 'FDGetter' C++ application in the src/main/cpp folder. You'll need the dbus-cxx library: https://github.com/dbus-cxx/dbus-cxx

The output of the C++ application will show the following:

~/dbus-java-filedescriptor/src/main/cpp/build$ ./FDGetter 
GOT FD 7
error: Bad file descriptor

dbus-monitor shows the following:

method call time=1576426826.425504 sender=:1.3215 -> destination=test.filedescriptor serial=2 path=/; interface=test.ReturnFDInterface; member=getFileDescriptor
method return time=1576426826.474387 sender=:1.3213 -> destination=:1.3215 serial=5 reply_serial=2
   file descriptor
         inode: 5941841
         type: fifo

Here's an example of sending filedescriptors using DBus-cxx: https://github.com/dbus-cxx/dbus-cxx/tree/321f4d4a76c6c47eb0e8bf18e5018d14fa88e30f/examples/basics/filedescriptors

Looking at the data that is coming back and forth, nothing seems obviously wrong, but clearly the FD is not getting sent properly.

from dbus-java.

After some more investigation, I think that the sizes that are set for the ancillary data are incorrect. Output from the Java side:

msg_control=[
    cmsg {
      cmsg_len=24
      cmsg_level=1
      cmsg_type=1
      cmsg_data=java.nio.HeapByteBuffer[pos=0 lim=8 cap=8]
    }
  ],

I did a quick test on the C++ side to figure out the correct sizes, and came up with the following:

sizeof(cmsghdr->cmsg_len) is 8
sizeof(cmsghdr->cmsg_level) is 4
sizeof(cmsghdr->cmsg_type) is 4
cmsg len is 20
controllen is 24

So the cmsg_len and the data length are off by four bytes; the controllen does need to be 24 bytes in length total, due to alignment. Probably this just needs to be fixed in BaseCmsgHdr, I will check tomorrow.

from dbus-java.

So I did check the other day if it was the number of bytes that were incorrect, but fixing that did not seem to change anything(e.g. making sure that the cmsg length and the controllen were 20 bytes, not 24). I spent some time messing around with it, and I haven't been able to figure out exactly what is broken. Whenever I try to send the FD, I get an EBADF back, which doesn't make sense since the FD that we are using to send data on is the same one that we've been using earlier in the program.

At this point, I feel that we need an even more basic example of a unix domain socket(without dbus) just to validate that we can send FDs in the first place.

from dbus-java.

I agree, first we should ensure that jnr-posix and friends is working like they should.

from dbus-java.

It seems that jnr-posix is not working correctly, but I have no idea why. Here's my latest; the Java and C programs should be equivalent.

The C program is from this StackOverflow response, and seems to work fine(or at least doesn't have an error) when I run. I was having issues with the fork() call in Java, so at the moment the Java and C programs simply try to send a filedescriptor to themselves. The C program doesn't get an error; the Java program always gets EBADF as the return from sendmsg when there is control data. Any thoughts?

from dbus-java.

I also did some testing and created a new repo here: https://github.com/hypfvieh/test-jnr-fd-passing
The C code is very ugly as I've copied it from various sources found in the internet (sorry I'm not a C programmer).

In my setup, I use some C code e.g. write to a unix socket which is read by Java code.

When java is reading and C is writing, I can see that 1 byte is written and 1 byte is read, but the java code gets stuck when trying to read the control data on the message.

When doing it the other way around, java is writing/C is reading, I don't get any control data as well.
The C code detects that there is a new "client connection", and starts recvmsg call, but this never returns.
This really looks like jnr-posix is doing something wrong ...

from dbus-java.

Update on my tests:
Reading with C and sending with java is now working like expected.

Reading with java and writing with C still gets stuck when trying to access the control data of the message...

from dbus-java.

I got a working sample setup!

Sending from C to Java and vice versa works.
Sending from Java to Java is also working!

Now we have to investigate what's wrong with the implementation in FDMessageReader/Writer

from dbus-java.

Okay so it looks like there were two problems: not calling flip on the byte buffer, and for some reason I was subtracting 1 from the FD on the CPP side. Sending file descriptors now works! I also made sure the endianess was correct, but that didn't seem to affect anything.

See: rm5248/dbus-java-filedescriptor@9794def
rm5248@0c92fc9

from dbus-java.

Update on the FDs: sending via the MethodReturn works, MethodCall is broken, but I think that I know what is causing it, if not how to fix it(yet).

What I'm getting on the Java side when I try to get a file descriptor as a method parameter:

[2019-12-29 14:49:55] DEBUG org.freedesktop.dbus.FDMessageReader - have 0 filedescriptors with incoming message msghdr {
  msg_name=null,
  msg_namelen=0,
  msg_iov=[
    iovec {
      iov_base=jnr.ffi.provider.jffi.DirectMemoryIO[address=0x7fb81c10bf40],
      iov_len=164,
    }
  ],
  msg_control=[
    cmsg {
      cmsg_len=28
      cmsg_level=1
      cmsg_type=2
      cmsg_data=java.nio.HeapByteBuffer[pos=0 lim=12 cap=12]
    },
    cmsg {
      cmsg_len=32
      cmsg_level=0
      cmsg_type=0
      cmsg_data=java.nio.HeapByteBuffer[pos=0 lim=16 cap=16]
    }
  ],
  msg_controllen=56
  msg_iovlen=1,
  msg_flags=0,
}

It seems that some part of jnr-posix is not doing some calculation correclty, since the two auxillary sections have lengths of 28 and 32 respectively, which does not add up to the total control length of 56. So I think that we're simply off by 4 bytes someplace, which would also conveniently explain the cmsg_len=32, as the file descriptor is number 32 after it gets passed to the second process.

from dbus-java.

It's working! You'll need this commit for jnr-posix, and the latest from my filedescriptor-passing branch.

Now that this works appropriately, we should be able to actually make some tests that work properly, although we will have to make sure that these updates get into jnr-posix, since that is critical for this to work properly.

from dbus-java.

Congrats! Nice work!

I guess it would be best if you could create a pull request for this project and also send a pull request to the maintainer of jnr-posix. Normally he is pretty quick with merging.
I have had issued a pull request because of missing SO_PASSCRED in jnr-unixsocket, this one was merged the very same day I issued the request.

from dbus-java.

The jnr-posix part has been merged; I'll make a clean PR in a few days to finally fix this.

from dbus-java.

I just went and was starting a cleanup of the code, however some of the tests are failing due to a JVM crash. Could you take a look and see if you also get a crash on your end? This looks suspiciously like an issue that I've seen before where too fast of memory allocation causes libc to crash the process.

from dbus-java.

The 'StressTest' is always failing on my machine. The crash dump of the JVM points to libc.

I agree that this issue is related to memory allocation.
As far as I understand ByteBuffer.allocateDirect will allocate memory outside of the JVM heap. Freeing this memory will be done by the Cleaner thread of DirectByteBuffer when the garbage collection is triggered.
Garbage collection might not be triggered (or triggered too late) when running the unit tests.

Most articles I've found for DirectByteBuffer suggests that the buffer should be allocated once and be re-used (for performance and memory reason). The problem is that, except for the header read in the beginning, the size for the buffer is unknown until we have read the header. So early and one-time allocation will not work.

Manually freeing the memory is also not really supported. There are some sample codes for that, but they will not work with Java >= 9 (access to sun package is forbidden).

from dbus-java.

Good to know that you see that issue as well; I also see it in the stress test. I think that what has to happen at the moment is that we'll simply have to allocate enough space and then expand as needed, or else modify the API to allow for a ByteBuffer. I'll see about coding this up and see if that fixes the problem; otherwise there's something funny in JNR.

from dbus-java.

The good news at this point is that I can get very consistent crashes in the StressTest. The bad news is that due to the multithreaded nature of the test and the fact that it's happening with some JNR-managed code, I can't figure out where it's happening nor how to fix it at the moment.

The following error keeps being output to the surefire reports:

# Created at 2020-02-09T13:26:40.754
malloc(): unsorted double linked list corrupted

# Created at 2020-02-09T13:26:40.759
Aborted

When I've seen this before, sometimes it's been because of allocating memory too fast or calling a big function too fast(I've had it happen before where I had to declare variables that would normally be in a C function to be declared outside of the C function in order to not make glibc freak out and give this error or a similar one.)

The secondary problem as well is that this could be happening with libffi, which is part of what JNR uses to call the C functions.

Any thoughts on how we can debug this?

from dbus-java.

That is really difficult. I would try to reproduce the issue without all that dbus-stuff.
Starting with the unit test IOTest of jnr-posix.

I would then try to put this code in a large loop which will re-alloc the memory and stuff like the dbus-implementation does. If allocation is the issue, I would expect the test to crash like the StressTest does.

from dbus-java.

So the good news is that I can reproduce this and I've made a test for it: https://github.com/rm5248/jnr-posix/tree/stress-test

The bad news is that it is incredibly picky about crashing; sometimes it crashes immediately with the double-free error, sometimes the JVM will actually pick up the crash, sometimes it runs fine. I originally thought it had something to do with the toString method, but that doesn't seem to actually be the case, although that may exacerbate the issue.

from dbus-java.

Uff...
For me it sounds like it's time to open an issue on jnr-posix referencing your unit test and describing what you've seen. Maybe the jnr guys can help to find the issue in their code.

All of theses jnr/ffi stuff is like magic to me, I'm sorry that I can't help here...

from dbus-java.

Given my experience with errors with malloc/glibc before, I haven't been able to come up with a plausible way of fixing the problem that doesn't involve allocating less memory. So I've started on a new branch that just reverts back to using JNI to handle the send/recv of the data: rm5248@0d8321a

My idea is that there will be multiple providers of MessageReader and MessageWriter that you can switch between, controlled by a Java environment variable, but using a sane default. Example:

java -cp *.jar -Dorg.freedesktop.dbus.MessageHandler=org.freedesktop.dbus.jniconnector....

By putting this into a separate module as well, we can keep the JNR code as-is, and you only need to bring in the JNI connector if you need the extra capability of passing the FileDescriptors. The default behavior would be to look on the classpath, and initialize the message handler with the JNI version if it exists and can load the library, else it will fallback to the current JNR version.

from dbus-java.

I'm not a friend of JNI especially for libraries like dbus-java.

I've had the libmatthew JNI wrapper library before and it was really a pain to provide native compiled libraries for all those different platforms (PC/Linux/MacOS/Raspberry in different versions flavors and other IOT devices). That's why I wanted to get rid of this and why I'm not interested in adding another native library (even in a sub module or something).

I would agress to provide different implementations for MessageReader/MessageWriter, so one can choose one or write some if required.
But I don't like the system property stuff. I would prefer to use the Java Service Provider interface and if there is a library providing alternative implementation of MessageReader/Writer dbus-java should use that one instead of the internal default.

That would allow me to continue the development of the current implementation and you can provide an extension which provides file descriptor stuff using JNI.

What do you think of that?

from dbus-java.

That sounds good to me and is generally what I was thinking. I was thinking of using the ServiceProvider as well, and that was the reasoning behind the system property: if you have multiple providers of MessageReader/MessageWriter, how do you select between them? To your point, we could simply ignore the internal version if we find at least one non-default implementation, so maybe that's not a big deal.

One potential problem that I see though is that there's two ways to implement the service: either with an InputStream/OutputStream, or by passing a file descriptor. Each implementation would need to have one or the other set, so there may be some useless methods that we need to define but that shouldn't cause a big issue.

from dbus-java.

I've added a new branch which uses Java SPI (https://github.com/hypfvieh/dbus-java/tree/dbus-spi).

The service provider must provide a implementation of org.freedesktop.dbus.spi.ISocketProvider.

I thought about the methods we may need to have. I think it should be suitable to have methods to provide a reader and a writer and also to have methods which indicates if the implementation is capable of passing FDs.

For me it looks like it should be working by providing the socket object to the createReader/Writer methods. Both, the input/output streams as well as the channel/file descriptor is accessable by using the socket object.

Maybe you can have a look.

from dbus-java.

That looks pretty good to me - I'll work on my implementation here in the next day or two so we can test an actual implementation out.

from dbus-java.

Good news! I have what I believe to be a working implementation. Everything appears to be working at the moment, and I'm not getting any segfaults. There were a few minor fixes that I made to ensure that it loaded properly; see #100 for the fixes to dbus-java.

I haven't validated the sending and receiving of the file descriptors yet, but I don't expect that to be a big issue at the moment. I will need to make a test for that.

from dbus-java.

Nice. I merged your changes to the spi branch. If you have finished testing, I'll merge it to master.

from dbus-java.

This all looks good to me at this point - I'll try to do some testing over the coming weeks, although the project that I actually needed this for may be cancelled at this point(hence why it has taken a long time to do this).

from dbus-java.

I finally managed to get around to finishing up the implementation on my end and upload to Maven Central: https://github.com/rm5248/dbus-java-nativefd

To use, simply add the following dependency to your pom.xml. The ServiceLoader will pick up the native implementation and use it instead of the default implementation for sending and receiving.

Note that this only works on Linux; if you try to use this on Windows, BSD, or OSX it will probably not work correctly.

<dependency>
    <groupId>com.rm5248</groupId>
    <artifactId>dbus-java-nativefd</artifactId>
    <version>1.0</version>
</dependency>

If you have issues using this dependency, open an issue on my repo and I can take a look at it.

from dbus-java.