Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: xorg-server/xorg-server-vfb/xwayland: A problem with shm support [the problem is localized]. #15239

Closed
twaik opened this issue Feb 16, 2023 · 10 comments · Fixed by #15288
Labels
bug report Something is not working properly x11 Issue is related to stuff requiring X11 environment or x11-packages

Comments

@twaik
Copy link
Member

twaik commented Feb 16, 2023

Problem description

Hi. The following code works on pc's Xvfb, but returns BadMatch on termux's Xvfb.

code
/*
gcc ss.c -o ss -lxcb -lxcb-shm -DANDROID=0 && DISPLAY=:0 ./ss
*/

#include <cstdio>
#include <cstring>
#include <unistd.h>
#include <xcb/xcb.h>
#include <xcb/shm.h>
#include <cstdlib>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <sys/mman.h>

const char *xcb_errors[19] = {
        "Success",
        "BadRequest",
        "BadValue",
        "BadWindow",
        "BadPixmap",
        "BadAtom",
        "BadCursor",
        "BadFont",
        "BadMatch",
        "BadDrawable",
        "BadAccess",
        "BadAlloc",
        "BadColor",
        "BadGC",
        "BadIDChoice",
        "BadName",
        "BadLength",
        "BadImplementation",
        "Unknown"
};

inline void handle_xcb_error(xcb_generic_error_t* err, int line) {
    if (!err)
        return;
    uint (*min)(uint, uint) = [](uint a, uint b) { return a < b ? a : b; };
    uint clamped_error_code = min(err->error_code, (sizeof(xcb_errors) / sizeof(xcb_errors[0])) - 1);

    printf("XCB error: %d (%s), sequence: %d, resource id: %d, major code: %d, minor code: %d on line %d\n",
         int(err->error_code), xcb_errors[clamped_error_code],
         int(err->sequence), int(err->resource_id),
         int(err->major_code), int(err->minor_code), line);
}

#define ASHMEM_SET_SIZE _IOW(0x77, 3, size_t)

static inline int
os_create_anonymous_file(size_t size) {
    int fd, ret;
    long flags;
    fd = open("/dev/ashmem", O_RDWR | O_CLOEXEC);
    if (fd < 0)
        return fd;
    ret = ioctl(fd, ASHMEM_SET_SIZE, size);
    if (ret < 0)
        goto err;
    flags = fcntl(fd, F_GETFD);
    if (flags == -1)
        goto err;
    if (fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == -1)
        goto err;
    return fd;
    err:
    close(fd);
    return ret;
}

xcb_screen_t* xcb_get_screen(xcb_connection_t* connection) {
    const xcb_setup_t* setup = xcb_get_setup(connection);  // I think we don't need to free/destroy this!
    return xcb_setup_roots_iterator(setup).data;
}

#define ERR_X_EXTENSION "Extension \"%s\" isn't available"

#define ERR_SHM_ALLOC "Failed to allocate shared memory"
#define ERR_SHM_ATTACH "Failed to attach shared memory"

#define MIT_SHM_EXTENSION_NAME "MIT-SHM"

#define PANIC(...) { printf(__VA_ARGS__); printf("\n"); abort(); }

int main(){
    // Connect to the X server
    xcb_connection_t* connection = xcb_connect(nullptr, nullptr);
    xcb_screen_t* screen         = xcb_get_screen(connection);

    xcb_query_extension_cookie_t extension_cookie = xcb_query_extension(connection, strlen(MIT_SHM_EXTENSION_NAME), MIT_SHM_EXTENSION_NAME);
    xcb_query_extension_reply_t *extension_reply =
            xcb_query_extension_reply(connection, extension_cookie, nullptr);
    if (extension_reply == nullptr)
        PANIC(ERR_X_EXTENSION, MIT_SHM_EXTENSION_NAME)
    free(extension_reply);
    xcb_shm_query_version_cookie_t version_cookie =
            xcb_shm_query_version(connection);
    xcb_shm_query_version_reply_t *version_reply =
            xcb_shm_query_version_reply(connection, version_cookie, nullptr);
    if (version_reply == nullptr)
        PANIC(ERR_X_EXTENSION, MIT_SHM_EXTENSION_NAME)
    free(version_reply);

    off_t max_size = screen->width_in_pixels * screen->height_in_pixels * 4;

//    int32_t shmid = shmget(IPC_PRIVATE, max_size, IPC_CREAT | 0777);
#if ANDROID
    int32_t shmfd = os_create_anonymous_file(max_size);
#else
    int32_t shmfd = shm_open("asdasd", O_CREAT | O_RDWR,S_IRUSR | S_IWUSR);
    ftruncate(shmfd, max_size);
#endif
//	if (shmid < 0)
    if (shmfd < 0)
        PANIC(ERR_SHM_ALLOC)
    //auto shmaddr = static_cast<uint8_t *>(shmat(shmid, nullptr, 0));
    auto shmaddr = static_cast<uint8_t *>(mmap(nullptr, max_size,PROT_READ | PROT_WRITE,MAP_SHARED, shmfd, 0));
    if (shmaddr == (uint8_t *) -1)
        PANIC(ERR_SHM_ATTACH)

    xcb_shm_seg_t shmseg = xcb_generate_id(connection);
    xcb_void_cookie_t attach_cookie = xcb_shm_attach_fd_checked(connection, shmseg, shmfd, 0);
    //xcb_void_cookie_t attach_cookie = xcb_shm_attach_checked(connection, shmseg, shmid, 0);
    xcb_generic_error_t *err =xcb_request_check(connection, attach_cookie);
    if (err) {
        handle_xcb_error(err, __LINE__);
        PANIC(ERR_SHM_ATTACH)
    }

    uint32_t events = XCB_EVENT_MASK_EXPOSURE | XCB_EVENT_MASK_KEY_PRESS | XCB_EVENT_MASK_KEY_RELEASE;
    xcb_window_t win = xcb_generate_id(connection);
    xcb_create_window (connection, screen->root_depth, win, screen->root, 0, 0, 800, 600,
                       10, XCB_WINDOW_CLASS_INPUT_OUTPUT, screen->root_visual, XCB_CW_EVENT_MASK, &events);

    xcb_gcontext_t default_gc = xcb_generate_id (connection);
    uint32_t value[]  = { screen->white_pixel, screen->black_pixel };
    xcb_create_gc(connection, default_gc, win, XCB_GC_FOREGROUND | XCB_GC_BACKGROUND, value);
    xcb_map_window(connection, win);
    xcb_flush(connection);
    for (int i = 0; i < 100; i++) {
        xcb_shm_get_image_cookie_t cookie =
                xcb_shm_get_image(connection, screen->root, 0, 0, screen->width_in_pixels, screen->height_in_pixels, ~0, XCB_IMAGE_FORMAT_Z_PIXMAP, shmseg, 0);
        xcb_shm_get_image_reply(connection, cookie, nullptr);

        xcb_shm_put_image(connection, win, default_gc, screen->width_in_pixels, screen->height_in_pixels, 0, 0, screen->width_in_pixels, screen->height_in_pixels, 0, 0, screen->root_depth, XCB_IMAGE_FORMAT_Z_PIXMAP, 1, shmseg, 0);

        xcb_flush(connection);
        usleep(100000);
    }

    xcb_disconnect(connection);
}
XCB error: 8 (BadMatch), sequence: 4, resource id: 0, major code: 130, minor code: 6 on line 131
Failed to attach shared memory
Aborted

If it will help I have xscope log
log.txt

What steps will reproduce the bug?

Compile this code (gcc ss.c -o ss -lxcb -lxcb-shm -DANDROID=1), start Xvfb (Xvfb :0 -ac) and then start ss (DISPLAY=:0 ./ss).

What is the expected behavior?

Program should not fail.

System information

termux-info:

Termux Variables:
TERMUX_APP_PACKAGE_MANAGER=apt
TERMUX_MAIN_PACKAGE_FORMAT=debian
TERMUX_VERSION=0.118.0
Packages CPU architecture:
aarch64
Subscribed repositories:
# sources.list
deb https://termux.librehat.com/apt/termux-main/ stable main
# x11-repo (sources.list.d/x11.list)
deb https://packages-cf.termux.dev/apt/termux-x11/ x11 main
Updatable packages:
All packages up to date
termux-tools version:
1.37.0
Android version:
12
Kernel build information:
Linux localhost 4.19.188-25223464-abA042FXXU1AVJ4 #1 SMP PREEMPT Thu Oct 20 15:35:01 KST 2022 aarch64 Android
Device manufacturer:
samsung
Device model:
SM-A042F
LD Variables:
LD_LIBRARY_PATH=
LD_PRELOAD=/data/data/com.termux/files/usr/lib/libtermux-exec.so
@twaik twaik added bug report Something is not working properly untriaged labels Feb 16, 2023
@xtkoba xtkoba added the x11 Issue is related to stuff requiring X11 environment or x11-packages label Feb 17, 2023
@xtkoba
Copy link
Contributor

xtkoba commented Feb 17, 2023

OK, this also reproduces with tigervnc, which uses xorg-server-xvfb as a (main) component.

@xtkoba xtkoba removed the untriaged label Feb 17, 2023
@twaik
Copy link
Member Author

twaik commented Feb 19, 2023

Well, it looks like xorg-server is right. But xcb is fault. The same snippet works right in proot. What can we do in this case?

@twaik
Copy link
Member Author

twaik commented Feb 19, 2023

Ok. 130 is a code of MIT-SHM. 6 is an opcode of SHM_ATTACH_FD. Everything should be fine, but for some reason request is broken. Xscope even did not recognise this request (it should go with sequence number: 0004, but it is simply skipped). As I see that is the reason why request fails with error. link. It is simply can not read fd. Xcb should do that only when it was built without HAVE_SENDMSG flag (link). But according to config.h it is present. It is so weird...

@xtkoba
Copy link
Contributor

xtkoba commented Feb 19, 2023

FWIW, the test case seems to work when using a custom shm_open (borrowed from some patch for some package) and with -DANDROID=0.

Also FWIW, this reminds me of log message such as follows when invoking some Qt application:

qt.qpa.xcb: xcb_shm_create_segment() failed for size 1936

The shm support in xcb seems to be broken in some way.

@xtkoba xtkoba changed the title [Bug]: Something is wrong with Xvfb [Bug]: libxcb: Something is wrong with shm support Feb 19, 2023
@twaik
Copy link
Member Author

twaik commented Feb 19, 2023

@twaik
Copy link
Member Author

twaik commented Feb 19, 2023

Why does it work with i.e. libwayland but fails to work with xtrans? Is it possible that sendmsg/recvmsg is somehow broken in bionic but fine in glibc?

@twaik
Copy link
Member Author

twaik commented Feb 19, 2023

I was wrong. glibc version with ANDROID=1 does not work. I did a mistake.

@twaik
Copy link
Member Author

twaik commented Feb 20, 2023

I found a root of problem. It is right here.

    if (fstat(fd, &statb) < 0 || statb.st_size == 0) {
        close(fd);
        return BadMatch;
    }

/dev/ashmem does not support getting size using fstat, (st_size always equals 0), only like this:

#define __ASHMEMIOC 0x77
#define ASHMEM_SET_SIZE _IOW(__ASHMEMIOC, 3, size_t)

int ashmem_get_size_region(int fd)
{
  return ioctl(fd, ASHMEM_GET_SIZE, NULL);
}

Are there any other ways to create shared memory file descriptor without implicitly open file on disk?

@twaik
Copy link
Member Author

twaik commented Feb 20, 2023

It is possible to patch Xorg to support ashmem_get_size_region if st_size == 0, bit it will not allow us to use it with prooted/chrooted xserver.

@twaik twaik changed the title [Bug]: libxcb: Something is wrong with shm support [Bug]: xorg-server/xorg-server-vfb/xwayland: A problem with shm support [the problem is localized]. Feb 20, 2023
@twaik
Copy link
Member Author

twaik commented Feb 20, 2023

Ok, I asked for proot support for this. We can patch xorg-server to support this behaviour. And finally I can patch os_create_anonymous_file to create anonymous file in /dev/tmp in the case if a process is running as root.
And it will handle 3 cases:

  1. When user runs termux's Xorg it will work as expected because termux's xorg-server supports this.
  2. When user runs proot Xorg it will work as expected because termux's proot will support this.
    3.1 When user runs Xorg in chrooted environment he will be asked to mount/create/something /dev/tmp to allow my program create regular files in some tmpfs and let my program work as expected.
    3.2. I can write small Xorg extension to support this and not drop file descriptor... But it will require user to modify config files...

But anyway work with chroot will require some additional steps from user...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Something is not working properly x11 Issue is related to stuff requiring X11 environment or x11-packages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants