beardypig / guncon3

Linux kernel driver for the Guncon3 light gun from Namco
https://www.beardypig.com/2016/01/06/guncon3/
GNU General Public License v2.0
33 stars 7 forks source link

guncon3_decode correction #4

Closed pcnimdock closed 5 years ago

pcnimdock commented 5 years ago

I have corrected the error of the random numbers when the stick is placed in certain positions. I signed the variables, a_sum,b_sum and key_offset

static int guncon3_decode(unsigned char *data, const unsigned char *key) {
    int x, y, key_index;
    unsigned char bkey, keyr, byte;
    s32 a_sum, b_sum;
    s32 key_offset;

    b_sum = ((data[13] ^ data[12]) + data[11] + data[10] - data[9] - data[8]) ^ data[7];
    b_sum&=0xFF;
    a_sum = (((data[6] ^ b_sum) - data[5] - data[4]) ^ data[3]) + data[2] + data[1] - data[0];
    a_sum &=0xFF;
    if (a_sum != key[7]) {
        if (debug)
            printk(KERN_ERR "checksum mismatch: %02x %02x\n", a_sum, key[7]);
        return -1;
    }

    key_offset = (((((key[1] ^ key[2]) - key[3] - key[4]) ^ key[5]) + key[6] - key[7]) ^ data[14]) + (unsigned char)0x26;
    key_offset &= 0xFF;
    key_index = 4;

    //byte E is part of the key offset
    // byte D is ignored, possibly a padding byte - make the checksum workout
    for (x = 12; x >= 0; x--) {
        byte = data[x];
        for (y = 4; y > 1; y--) { // loop 3 times
            key_offset--;

            bkey = KEY_TABLE[key_offset + 0x41];
            keyr = key[key_index];
            if (--key_index == 0)
                key_index = 7;

            if ((bkey & 3) == 0)
                byte =(byte - bkey) - keyr;
            else if ((bkey & 3) == 1)
                byte = ((byte + bkey) + keyr);
            else
                byte = ((byte ^ bkey) ^ keyr);
        }
        data[x] = byte;
    }
    return 0;
}
beardypig commented 5 years ago

Cool! I don't have the Guncon 3 with me at the moment, but it seems like this change would be sufficient without the need to mask the LSB in the result.

diff --git a/guncon3.c b/guncon3.c
index e845930..744aa3b 100644
--- a/guncon3.c
+++ b/guncon3.c
@@ -104,7 +104,9 @@ MODULE_DEVICE_TABLE(usb, usb_guncon3_id_table);

 static int guncon3_decode(unsigned char *data, const unsigned char *key) {
     int x, y, key_index;
-    unsigned char bkey, keyr, a_sum, b_sum, key_offset, byte;
+    unsigned char bkey, keyr, byte;
+    char a_sum, b_sum, key_offset;
+
     b_sum = ((data[13] ^ data[12]) + data[11] + data[10] - data[9] - data[8]) ^ data[7];
     a_sum = (((data[6] ^ b_sum) - data[5] - data[4]) ^ data[3]) + data[2] + data[1] - data[0];
     if (a_sum != key[7]) {

If you are able to test these changes I would gladly accept a pull request :)

pcnimdock commented 5 years ago

I tested the patch with Ubuntu 17.04 with kernel Linux 4.10.0-42-generic. It 's working well now

beardypig commented 5 years ago

Thanks for the tip, I made the change in dd7ad767233b2404fca54886c5d4c9af432a4108.