[PATCH] Don't count RetainPermanent clients twice in security.c

[Peter Harris]

On 2012-11-23 16:44, Peter Harris wrote:
> If a RetainPermanent client is subsequently killed by a KillClient
> request, the reference count is decremented twice. This can cause the
> server to prematurely kill other clients using the same Authorization.

Attached is a simple app to demonstrate the problem. The window should
stay up forever (or until manually killed), but after 60 seconds (the
default security Auth timeout) the window disappears.

In the real world, the activity isn't all done by the same application.
The RetainPermanent/KillClient cycle is usually different apps separated
in time fighting over the root window (eg. xsetroot killing a previous
xsetroot), which can cause unrelated windows to disappear seemingly at
random.

Peter Harris
-- 
               Open Text Connectivity Solutions Group
Peter Harris                    http://connectivity.opentext.com/
Research and Development        Phone: +1 905 762 6001
pharris at opentext.com            Toll Free: 1 877 359 4866
-------------- next part --------------
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <xcb/xcb.h>
#include <X11/Xlib.h>
#include <X11/extensions/security.h>

#define CYCLES 24

static int get_auth(xcb_auth_info_t *auth)
{
    Display *dpy;

    dpy = XOpenDisplay(NULL);
    if (!dpy) {
        fprintf(stderr, "Cannot open control display\n");
        return 1;
    }

    int major, minor;
    int rv = XSecurityQueryExtension(dpy, &major, &minor);
    if (!rv) {
        fprintf(stderr, "Cannot query SECURITY extension\n");
        return 1;
    }

    Xauth *in, *out;
    in = XSecurityAllocXauth();
    in->name = auth->name;
    in->name_length = auth->namelen;
    in->data = NULL;
    in->data_length = 0;
    XSecurityAuthorization id;
    XSecurityAuthorizationAttributes attr = { .trust_level = XSecurityClientTrusted };
    out = XSecurityGenerateAuthorization(dpy, in, XSecurityTrustLevel, &attr, &id);

    XSecurityFreeXauth(in);

    if (!out) {
        fprintf(stderr, "Cannot generate SECURITY cookie\n");
        return 1;
    }

    auth->datalen = out->data_length;
    auth->data = malloc(out->data_length);
    memcpy(auth->data, out->data, auth->datalen);
    XSecurityFreeXauth(out);

    return 0;
}

static xcb_screen_t *root_visual(xcb_connection_t *c, int scnum)
{
    const xcb_setup_t *s = xcb_get_setup(c);
    if (scnum > xcb_setup_roots_length(s))
        scnum = 0;
    xcb_screen_iterator_t it = xcb_setup_roots_iterator(s);
    for (int i=0; i < scnum; i++) {
        xcb_screen_next(&it);
    }
    return it.data;
}

int main(int argc, char *argv[])
{
    xcb_auth_info_t auth = { .name = "MIT-MAGIC-COOKIE-1" };
    auth.namelen = strlen(auth.name);
    int rv = get_auth(&auth);
    if (rv)
        return rv;

    int scnum;
    xcb_connection_t *c = xcb_connect_to_display_with_auth_info(NULL, &auth, &scnum);
    if (!c || xcb_connection_has_error(c)) {
        fprintf(stderr, "Cannot open dummy window connection\n");
        return 1;
    }

    xcb_screen_t *root = root_visual(c, scnum);

    xcb_window_t win = xcb_generate_id(c);
    uint32_t list[] = { root->black_pixel };
    xcb_create_window(c, XCB_COPY_FROM_PARENT, win, root->root,
            100, 100, 100, 100, // x, y, w, h
            0, XCB_WINDOW_CLASS_INPUT_OUTPUT, XCB_COPY_FROM_PARENT,
            XCB_CW_BACK_PIXEL, list);
    xcb_map_window(c, win);
    xcb_flush(c);

    printf("Doing RetainPermanent trick");
    fflush(stdout);
    xcb_window_t prev = 0;
    for (int i=0; i < CYCLES; i++) {
        xcb_connection_t *rp = xcb_connect_to_display_with_auth_info(NULL, &auth, &scnum);
        if (!rp || xcb_connection_has_error(rp)) {
            fprintf(stderr, "Cannot open default display on iteration %d\n", i);
            return 1;
        }

        if (prev)
            xcb_kill_client(rp, prev);
        prev = xcb_generate_id(rp);
        xcb_create_window(rp, XCB_COPY_FROM_PARENT, prev, root->root,
                10, 10, 10, 10, // x, y, w, h
                0, XCB_WINDOW_CLASS_INPUT_OUTPUT, XCB_COPY_FROM_PARENT,
                0, NULL);

        xcb_set_close_down_mode(rp, XCB_CLOSE_DOWN_RETAIN_PERMANENT);
        xcb_flush(rp);
        usleep(10 * 1000);
        xcb_disconnect(rp);

        printf(".");
        fflush(stdout);
        usleep(100 * 1000); // make sure server noticed close
    }

    printf("\nWaiting to see what happens\n");
    xcb_generic_event_t *ev;
    while (ev = xcb_wait_for_event(c)) {
        if (!ev || xcb_connection_has_error(c)) {
            printf("Connection Error.\n");
            return 2;
        }
        free(ev);
    }

    return 0;
}