2

I am working on an embedded Linux system (kernel-5.10.188), where adbd is running in the target system.

In the target system I created a file with echo "hello" > /data/open, then I ran following command in Windows PC.

PS C:\Users\aa> adb push C:\test\open /data/
C:\test\open: 1 file pushed. 0.7 MB/s (17096 bytes in 0.023s)

In the target system,

# ls /data/open -l
-rw-rw-rw-    1 root     root         17096 Mar 11 10:28 /data/open

The function of handle_send_file() in file_sync_service.c is as follows (I added 3 lines of printf)

static int handle_send_file(int s, char *path, mode_t mode, char *buffer)
{
     syncmsg msg;
     unsigned int timestamp = 0;
     int fd;

     printf("XXXXXX %s, path: %s, mode: %o\n", __func__, path, mode);
     fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL, mode);
     printf("XXXXX fd: %d, errno: %d\n", fd, errno);
     if(fd < 0 && errno == ENOENT) {
         mkdirs(path);
         fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL, mode);
     }
     printf("XXXXX fd: %d, errno: %d\n", fd, errno);
     if(fd < 0 && errno == EEXIST) {
         fd = adb_open_mode(path, O_WRONLY, mode);
     }
     if(fd < 0) {
         if(fail_errno(s))
             return -1;
         fd = -1;
     }

I got confused after reading the codes.

     fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL, mode);
     printf("XXXXX fd: %d, errno: %d\n", fd, errno);

I think if the /data/open exists, line 203 should return -1 with errno of EEXIST, and it should go to line 211 to open the file with O_WRONLY. But it did NOT!

But it showed the followings.

XXXXXX handle_send_file, path: /data/open, mode: 666
XXXXX fd: 17, errno: 0
XXXXX fd: 17, errno: 0
sync: waiting for command

I pushed open again from the PC, and got.

XXXXXX handle_send_file, path: /data/open, mode: 666
XXXXX fd: 17, errno: 0
XXXXX fd: 17, errno: 0
sync: waiting for command

There is NO error returned when the /data/open exists! Is this correct and expected?

The adb_open_mode() and relevant symbols are defined in sysdeps.h.

/*
  * TEMP_FAILURE_RETRY is defined by some, but not all, versions of
  * <unistd.h>. (Alas, it is not as standard as we'd hoped!) So, if it's
  * not already defined, then define it here.
  */
 #ifndef TEMP_FAILURE_RETRY
 /* Used to retry syscalls that can return EINTR. */
 #define TEMP_FAILURE_RETRY(exp) ({         \
     typeof (exp) _rc;                      \
     do {                                   \
         _rc = (exp);                       \
     } while (_rc == -1 && errno == EINTR); \
     _rc; })
 #endif

 static __inline__ int  adb_open_mode( const char*  pathname, int  options, int  mode )
 {
     return TEMP_FAILURE_RETRY( open( pathname, options, mode ) );
 }

I think it calls open, correct me if I am wrong.

Then, I wrote following codes with similar logics of adb_open_mode() to check its behavior when open an existing file of /data/open.

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>

int adb_open_mode(const char *path, int flags, mode_t mode) {
    return open(path, flags, mode);
}

void fake_mkdirs(const char *path) {
    printf("Creating directories for %s\n", path);
}

int main(int argc, char **argv)
{
    const char *path = "/tmp/testfile";
    /// mode_t mode = 0644;
    mode_t mode = 0666;
    int fd;

    if (argc != 2) {
        printf("Usage: %s filetoopen\n", argv[0]);
        exit(0);
    }

    path = argv[1];
    fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL, mode);
    printf("%s: %d, fd: %d, errno: %d\n", __func__, __LINE__, fd, errno);
    if (fd < 0 && errno == ENOENT) {
        fake_mkdirs(path);
        fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL, mode);
        printf("%s: %d, fd: %d, errno: %d\n", __func__, __LINE__, fd, errno);
    }
        printf("%s: %d, fd: %d, errno: %d\n", __func__, __LINE__, fd, errno);
    if (fd < 0 && errno == EEXIST) {
        fd = adb_open_mode(path, O_WRONLY, mode);
    }
    printf("%s: %d, fd: %d, errno: %d\n", __func__, __LINE__, fd, errno);

    if (fd < 0) {
        perror("Failed to open file");
    } else {
        printf("File opened successfully with fd %d\n", fd);
        close(fd);
    }

    return 0;
}

I ran it and got followings.

# ./a.out /data/open
main: 32, fd: -1, errno: 17
main: 38, fd: -1, errno: 17
main: 42, fd: 3, errno: 17
File opened successfully with fd 3

With pre-processing of file_sync_service.c.

I got this,

static __inline__ int adb_open_mode( const char* pathname, int options, int mode )
{
    return
# 344 "/home/t/android-tools-4.2.2+git20130218/core/adbd/sysdeps.h" 3 4
          (__extension__ ({ long int __result; do __result = (long int) (
# 344 "/home/t/android-tools-4.2.2+git20130218/core/adbd/sysdeps.h"
          open( pathname, options, mode )
# 344 "/home/t/android-tools-4.2.2+git20130218/core/adbd/sysdeps.h" 3 4              ); while (__result == -1L && (*__errno_location ()) == 4); __result; }))
# 344 "/home/t/android-tools-4.2.2+git20130218/core/adbd/sysdeps.h"
                                                               ;
}

It seemd that adbd is using open directly.

Then, I tried strace -p \pidof adbd` -ff -o /tmp/a.log, then adb push C:/inotify /data/, I got the log about the adb push`.

write(1, "sync: waiting for command\n", 26) = 26
read(15, "STAT\6\0\0\0", 8)             = 8
read(15, "/data/", 6)                   = 6
statx(AT_FDCWD, "/data/", AT_STATX_SYNC_AS_STAT|AT_SYMLINK_NOFOLLOW|AT_NO_AUTOMOUNT, STATX_BASIC_STATS, {stx_mask=STATX_BASIC_STATS|0x1000, stx_at
write(15, "STAT\375A\0\0`\22\0\0A\t\0\0", 16) = 16
write(1, "sync: waiting for command\n", 26) = 26
read(15, "SEND\23\0\0\0", 8)            = 8
read(15, "/data/inotify,33206", 19)     = 19
unlink("/data/inotify")                 = 0
write(1, "XXXXXX handle_send_file, path: /"..., 56) = 56
openat(AT_FDCWD, "/data/inotify", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE, 0666) = 16
write(1, "XXXXX fd: 16, errno: 0\n", 23) = 23
write(1, "XXXXX fd: 16, errno: 0\n", 23) = 23
....

It showed adbd called unlink("/data/inotify"); firstly before open and write.

That makes it clear.

9
  • Is adb_open documented to return an error in errno? Commented Mar 12 at 0:38
  • 1
    The question title is misleading. It seems to say the question is about open(), but it most directly about adb_open_mode(), which is not the same thing. Commented Mar 12 at 0:57
  • let me update the question with adb_open_mode() to make it NOT misleading. Commented Mar 12 at 0:58
  • Given that adb_open_mode() in fact is not open(), what makes you think it will behave as open() would with the same arguments? I'm having trouble finding any docs at all, but maybe you have some information I don't. Commented Mar 12 at 1:00
  • I updated the quesion with adb_open_mode() codes and my testing codes for your reference. Commented Mar 12 at 1:06

1 Answer 1

2

With more tests I found the unexpected open behavior.

The short answer is 'It is NOT unexpected'.

From the log of strace adbd, it showed that adbd deleted the file to be pushed in the target directory before calling adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL, mode); to create the file.

It is from the following codes in file_sync_service.c

static int do_send(int s, char *path, char *buffer)
{
    char *tmp;
    mode_t mode;
    int is_link, ret;

    tmp = strrchr(path,',');
    if(tmp) {
        *tmp = 0;
        errno = 0;
        mode = strtoul(tmp + 1, NULL, 0);
#ifndef HAVE_SYMLINKS
        is_link = 0;
#else
        is_link = S_ISLNK(mode);
#endif
        mode &= 0777;
    }
    if(!tmp || errno) {
        mode = 0644;
        is_link = 0;
    }

    adb_unlink(path); //// << delete target file before creation.


#ifdef HAVE_SYMLINKS
    if(is_link)
        ret = handle_send_link(s, path, buffer);
    else {
#else
    {
#endif
        /* copy user permission bits to "group" and "other" permissions */
        mode |= ((mode >> 3) & 0070);
        mode |= ((mode >> 3) & 0007);

        ret = handle_send_file(s, path, mode, buffer);
    }

By removing the adb_unlink(path), I saw the expected open behavior as follows,

sync: waiting for command
XXXXXX handle_send_file, path: /tmp/open, mode: 666
XXXXX fd: 16, errno: 0
XXXXX fd: 16, errno: 0
sync: waiting for command


XXXXXX handle_send_file, path: /tmp/open, mode: 666
XXXXX fd: -1, errno: 17
XXXXX fd: -1, errno: 17
sync: waiting for command

XXXXXX handle_send_file, path: /tmp/open, mode: 666
XXXXX fd: -1, errno: 17
XXXXX fd: -1, errno: 17
sync: waiting for command

XXXXXX handle_send_file, path: /tmp/open, mode: 666
XXXXX fd: -1, errno: 17
XXXXX fd: -1, errno: 17
sync: waiting for command

So the unexpected behavior is from the unlink before open. and adb_open_mode() works as expected.

Sign up to request clarification or add additional context in comments.

5 Comments

Note: both unlink-before-open and plain open are almost always buggy if the writing process is interrupted in any way. Normally you should write to a new file in the same directory, perform the appropriate filesystems incantations, then atomically replace the target file. This does not apply if using O_APPEND or if you are paid big money to write database engines.
what do you mean if the writing process is interrupted? adb_open_mode() wraps the call of open with TEMP_FAILURE_RETRY to handle the error of EINTR. And how to atomically replace the target file?
And this (the problem isn't what you think it is) is among the reasons why we usually want a minimal reproducible example. I think the reason we didn't ask for one in this case is that it was unclear that you hadn't already given one.
Don't forget "the process crashes" and "the device loses power" are both common things that you need to be resilient against. (on a deeper level, "storage device silently starts bitrotting" requires a hash (above or below the filesystem level), and "storage device is bricked" is much more complicated to handle)
As for MRE John mentioned, I provided what I had when I asked the question and updated it with what I found with the comments to the question (thank all). If I found the unlink(path) before the open, I won't post this question. Sometimes a solid answer is impossible for the lack of details/context/MRE, but the comments like Employed Russian gave is helpful, it is a tip not an real answer, and it gave the right direction to NOT doubt adb_open_mode. As for "process crashes", it is possible in real case, sometimes it can be fixed by retrying, sometimes not, it is a case by case.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.