indigo-astronomy / indigo

INDIGO is a system of standards and frameworks for multiplatform and distributed astronomy software development designed to scale with your needs.
http://www.indigo-astronomy.org
Other
139 stars 64 forks source link

ccd_ptp: indigo_server crash when connecting Nikon cameras #482

Closed tail-feather closed 12 months ago

tail-feather commented 1 year ago

This problem only occurs in Raspberry Pi with INDIGO Sky.

Using image: 2023-04-09-indigosky-armhf.img Camera:

Crash while trying to decode ptp_property_nikon_MovScreenSize.

tail-feather commented 1 year ago

Crash point: https://github.com/indigo-astronomy/indigo/blob/e6958a8312af77493aee901411959ccefbd93292/indigo_drivers/ccd_ptp/indigo_ptp_nikon.c#L761

tail-feather commented 1 year ago

If the following properties are skipped, the connection is successful.

polakovic commented 1 year ago

About month ago I made one change on PTP driver which may affect it - unknown properties are not decoded to avoid a similar crash with Fuji XT5. I'll check if it may be related.

polakovic commented 1 year ago

I tested with D750 now and it worked and unused properties was not decoded. Are you sure that INDIGO was already updated on that image to the most recent version? You need INDIGO 2.0-2.0-238 or later.

tail-feather commented 1 year ago

Yes. I built and tested master on a Raspberry Pi (INDIGO Sky).

I tried it on Ubuntu and did not have this problem. This issue seems to occur only on the Raspberry Pi...

tail-feather commented 1 year ago

I got dump of PTP response.

Apparently, the problem occurs when parsing 64-bit integers.

d0a0:
88 bytes, 6 lines
[0]: a0 d0 08 00 01 00 00 3c 00 38 04 80 07 00 00 1e 
[1]: 00 38 04 80 07 02 08 00 00 00 1e 00 70 08 00 0f 
[2]: 00 00 19 00 70 08 00 0f 00 00 18 00 70 08 00 0f 
[3]: 00 00 3c 00 38 04 80 07 00 00 32 00 38 04 80 07 
[4]: 00 00 1e 00 38 04 80 07 00 00 19 00 38 04 80 07 
[5]: 00 00 18 00 38 04 80 07 
------------------------------------------------------------------------
code: a0 d0 : d0a0
type: 08 00 : 0008 -> ptp_uint64_type
writable: 01 : true
skip: 00 00 3c 00 38 04 80 07
value: 00 00 1e 00 38 04 80 07 : 07800438001e0000
form: 02 -> ptp_enum_form
count: 08 00 : 0008
- 00 00 1e 00 70 08 00 0f 
- 00 00 19 00 70 08 00 0f
- 00 00 18 00 70 08 00 0f 
- 00 00 3c 00 38 04 80 07
- 00 00 32 00 38 04 80 07 
- 00 00 1e 00 38 04 80 07
- 00 00 19 00 38 04 80 07 
- 00 00 18 00 38 04 80 07 

d0e2:
22 bytes, 2 lines
[0]: e2 d0 08 00 00 01 00 00 00 00 00 00 00 00 00 00 
[1]: 00 00 00 00 00 00 
------------------------------------------------------------------------
code: e2 d0 : d0e2
type: 08 00 : 0008 -> ptp_uint64_type
writable: 00 : false
skip: 01 00 00 00 00 00 00 00
value: 00 00 00 00 00 00 00 00
form: 00 : ptp_none_form

d0e7:
22 bytes, 2 lines
[0]: e7 d0 08 00 00 00 00 00 00 00 00 00 00 80 00 00 
[1]: 00 00 00 00 00 00 
------------------------------------------------------------------------
code: e7 d0 : d0e7
type: 08 00 : 0008 -> ptp_uint64_type
writable: 00 : false
skip: 00 00 00 00 00 00 00 00
value: 80 00 00 00 00 00 00 00
form: 00  : ptp_none_form
tail-feather commented 1 year ago

And I got coredump.

Program terminated with signal SIGBUS, Bus error.
#0 ptp_decode_uint64(source=source@entry=0xf5518d05 "", target=0xf3cfd488, target@entry=0xf3cfd480) at indigo_ptp.c:463
643            *target = *(uint64_t *)source;

Is this an alignment issue on a 32-bit OS?

polakovic commented 1 year ago

Hm, this may be the issue. Pls. can you try to replace that line 463 with something like this?

memcpy(target, source, sizeof(uint64_t));

polakovic commented 12 months ago

I can reproduce the issue with the following code on 32 bit ARM system:

#include <stdio.h>
#include <stdint.h>

void main() {
        uint8_t i8[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
        uint64_t i64;

        for (int i = 0; i < 8; i++) {
                i64 = *((uint64_t *)(i8 + i));
                printf("%d %lx\n", i, i64);
        }
}

but it does work with a code like this

#include <stdio.h>
#include <stdint.h>
#include <string.h>

void main() {
        uint8_t i8[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
        uint64_t i64;

        for (int i = 0; i < 8; i++) {
                //i64 = *((uint64_t *)(i8 + i));
                memcpy(&i64, i8 + i, sizeof(uint64_t));
                printf("%d %lx\n", i, i64);
        }
}
polakovic commented 12 months ago

Pls. can you test it with the following change?

df0866842461dbe082551fb54d22b58da290baf0

tail-feather commented 12 months ago

It worked! Thank you!