LeechCore_Read requires local buffer alignment / size restrictions

[shuffle2]

I am using the following code to wrap LeechCore:

struct DmaDevice {
    ~DmaDevice() {
        LeechCore_Close();

        _aligned_free(bounce_page);
        bounce_page = {};
    }
    bool Open() {
        bounce_page = (u8*)_aligned_malloc(PAGE_SIZE, PAGE_SIZE);

        config = {};
        config.magic = LEECHCORE_CONFIG_MAGIC;
        config.version = LEECHCORE_CONFIG_VERSION;
        strcpy(config.szDevice, "USB3380");
        return LeechCore_Open(&config) == TRUE;
    }
    bool BounceRead(u8 *buf, pa_t pa, u32 len) {
        if (LeechCore_Read(pa, bounce_page, PAGE_SIZE) != PAGE_SIZE) {
            return false;
        }
        memcpy(buf, bounce_page, len);
        return true;
    }
    bool Read(void* buf, pa_t pa, u32 len) {
        // pcileech/USB3380 can't handle non-page aligned local addresses.
        // the transfer size must also be page multiple.
        auto p = (u8*)buf;
        while (len >= PAGE_SIZE) {
            if (!BounceRead(p, pa, PAGE_SIZE)) {
                return false;
            }
            p += PAGE_SIZE;
            pa += PAGE_SIZE;
            len -= PAGE_SIZE;
        }
        if (len) {
            if (!BounceRead(p, pa, len)) {
                return false;
            }
        }
        return true;
    }
    bool Write(pa_t pa, void* buf, u32 len) {
        return LeechCore_Write(pa, (u8*)buf, len) == TRUE;
    }
    LEECHCORE_CONFIG config{};
    u8* bounce_page{};
};

I found reading is unreliable if the given buffer address/size is not aligned to some relatively high power of 2 (not entirely sure 0x1000 bytes is required). I haven't debugged down LeechCore/whatever is underneath to support the USB3380 device, but wanted to note the issue here.

Perhaps this logic should replace: https://github.com/ufrisk/LeechCore/blob/e35b0cace0f0e4f18cffddf134f8c2c6824fffb7/leechcore/leechcore.c#L413 This logic in LeechCore_ReadEx doesn't seem to help for me, I've tried forcing both paths. It seems both buffer alignment and transfer size of the local destination buffer of the read has some specific requirements.

Windows Version 10.0.18362.295, using "Android ADB Interface" driver from 8/28/2016, which seems to use WinUSB.

[ufrisk]

Thanks, I'll look into this later this weekend. The USB3380 should be able to handle non-aligned reads, but it's not very well tested. As a workaround just do page-sized reads until I've fixed it.

Those parts of the code aren't very nice - I took a lot of old code and refactored it so it flows to the generic LeechCore_ReadScatter function rather than directly down to the device layer when I moved it into the LeechCore project. It doesn't surprise me that there are bugs. Anyway, I'll look into it.

[ufrisk]

Update: I'll completely redo the code in the leechcore.c (i.e. copy code from the memprocfs) to turn any unaligned smaller reads into page-sized, page-aligned reads.

example: if you read 2 bytes from address 0xfff - two pages will be read from 0x0 and 0x1000 and then the two bytes will be copied. The extra read size will have negligible performance impact since reads that small are mostly affected by latency anyway.

I hopefully have some new/changed hardware coming up, and will push the changes sometime in October for this one if everything goes well.

[shuffle2]

Cool, however remember that the thing needing alignment is the host buffer. The remote address (the place being read from) doesn't need alignment (at least not in my wrapper code, maybe something underneath already deals with target buffer alignment, idk)

On Oct 1, 2019, at 00:59, Ulf Frisk notifications@github.com wrote:

Update: I'll completely redo the code in the leechcore.c (i.e. copy code from the memprocfs) to turn any unaligned smaller reads into page-sized, page-aligned reads.

example: if you read 2 bytes from address 0xfff - two pages will be read from 0x0 and 0x1000 and then the two bytes will be copied. The extra read size will have negligible performance impact since reads that small are mostly affected by latency anyway.

I hopefully have some new/changed hardware coming up, and will push the changes sometime in October for this one if everything goes well.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[ufrisk]

I hope this should work better now.

[ufrisk]

I'm closing the issue since I believe this should now be resolved. If the problem persists please let me know and I'll look into it again.