Skip to content

Commit f22abab

Browse files
paullj1paullj1
committed
Fix complete annihilation of storage on new pair
- Might fix Mixiaoxiao#103, Mixiaoxiao#139, Mixiaoxiao#147, Mixiaoxiao#184, Mixiaoxiao#198 - Also adds changes by @ruleechen - Also adds changes by @thiti-y
1 parent c90fa32 commit f22abab

File tree

4 files changed

+84
-108
lines changed

4 files changed

+84
-108
lines changed

‎src/arduino_homekit_server.cpp

+17-60
Original file line numberDiff line numberDiff line change
@@ -524,27 +524,21 @@ void write(client_context_t *context, byte *data, int data_size) {
524524
CLIENT_ERROR(context, "The socket is null! (or is closed)");
525525
return;
526526
}
527+
if (context->disconnect) {
528+
context->error_write = true;
529+
return;
530+
}
527531
if (context->error_write) {
528532
CLIENT_ERROR(context, "Abort write data since error_write.");
529533
return;
530534
}
531535
int write_size = context->socket->write(data, data_size);
532536
CLIENT_DEBUG(context, "Sending data of size %d", data_size);
533537
if (write_size != data_size) {
534-
CLIENT_ERROR(context, "socket.write, data_size=%d, write_size=%d", data_size, write_size);
535538
context->error_write = true;
536-
// Error write when :
537-
// 1. remote client is disconnected
538-
// 2. data_size is larger than the tcp internal send buffer
539-
// But We has limited the data_size to 538, and TCP_SND_BUF = 1072. (See the comments on HOMEKIT_JSONBUFFER_SIZE)
540-
// So we believe here is disconnected.
541-
context->disconnect = true;
542-
homekit_server_close_client(context->server, context);
543-
// We consider the socket is 'closed' when error in writing (eg. the remote client is disconnected, NO tcp ack receive).
544-
// Closing the socket causes memory-leak if some data has not been sent (the write_buffer did not free)
545-
// To fix this memory-leak, add tcp_abandon(_pcb, 0); in ClientContext.h of ESP8266WiFi-library.
539+
context->socket->keepAlive(1, 1, 1); // fast disconnected internally in 1 second.
540+
CLIENT_ERROR(context, "socket.write, data_size=%d, write_size=%d", data_size, write_size);
546541
}
547-
548542
}
549543

550544
int client_send_encrypted_(client_context_t *context,
@@ -2699,9 +2693,6 @@ void homekit_server_on_reset(client_context_t *context) {
26992693

27002694
homekit_server_reset();
27012695
send_204_response(context);
2702-
2703-
//vTaskDelay(3000 / portTICK_PERIOD_MS);
2704-
27052696
homekit_system_restart();
27062697
}
27072698

@@ -3141,27 +3132,30 @@ void homekit_mdns_init(homekit_server_t *server) {
31413132

31423133
homekit_accessory_t *accessory = server->config->accessories[0];
31433134
homekit_service_t *accessory_info = homekit_service_by_type(accessory,
3144-
HOMEKIT_SERVICE_ACCESSORY_INFORMATION);
3135+
HOMEKIT_SERVICE_ACCESSORY_INFORMATION);
31453136
if (!accessory_info) {
31463137
ERROR("Invalid accessory declaration: no Accessory Information service");
31473138
return;
31483139
}
31493140

31503141
homekit_characteristic_t *name = homekit_service_characteristic_by_type(accessory_info,
3151-
HOMEKIT_CHARACTERISTIC_NAME);
3142+
HOMEKIT_CHARACTERISTIC_NAME);
3143+
31523144
if (!name) {
31533145
ERROR("Invalid accessory declaration: " "no Name characteristic in AccessoryInfo service");
31543146
return;
31553147
}
3156-
3148+
31573149
homekit_characteristic_t *model = homekit_service_characteristic_by_type(accessory_info,
3158-
HOMEKIT_CHARACTERISTIC_MODEL);
3150+
HOMEKIT_CHARACTERISTIC_MODEL);
3151+
31593152
if (!model) {
31603153
ERROR("Invalid accessory declaration: " "no Model characteristic in AccessoryInfo service");
31613154
return;
31623155
}
31633156

31643157
if (homekit_mdns_started) {
3158+
// MDNS.close();
31653159
MDNS.begin(name->value.string_value, staIP);
31663160
INFO("MDNS restart: %s, IP: %s", name->value.string_value, staIP.toString().c_str());
31673161
MDNS.announce();
@@ -3175,7 +3169,7 @@ void homekit_mdns_init(homekit_server_t *server) {
31753169
INFO("MDNS begin: %s, IP: %s", name->value.string_value, staIP.toString().c_str());
31763170

31773171
MDNSResponder::hMDNSService mdns_service = MDNS.addService(name->value.string_value,
3178-
HOMEKIT_MDNS_SERVICE, HOMEKIT_MDNS_PROTO, HOMEKIT_SERVER_PORT);
3172+
HOMEKIT_MDNS_SERVICE, HOMEKIT_MDNS_PROTO, HOMEKIT_SERVER_PORT);
31793173
// Set a service specific callback for dynamic service TXT items.
31803174
// The callback is called, whenever service TXT items are needed for the given service.
31813175
MDNS.setDynamicServiceTxtCallback(mdns_service,
@@ -3206,31 +3200,6 @@ void homekit_mdns_init(homekit_server_t *server) {
32063200
//MDNS.addServiceTxt(HAP_SERVICE, HOMEKIT_MDNS_PROTO, "sf", (server->paired) ? "0" : "1");
32073201
MDNS.addServiceTxt(mdns_service, "ci", String(server->config->category).c_str());
32083202

3209-
/*
3210-
// accessory model name (required)
3211-
homekit_mdns_add_txt("md", "%s", model->value.string_value);
3212-
// protocol version (required)
3213-
homekit_mdns_add_txt("pv", "1.0");
3214-
// device ID (required)
3215-
// should be in format XX:XX:XX:XX:XX:XX, otherwise devices will ignore it
3216-
homekit_mdns_add_txt("id", "%s", server->accessory_id);
3217-
// current configuration number (required)
3218-
homekit_mdns_add_txt("c#", "%d", server->config->config_number);
3219-
// current state number (required)
3220-
homekit_mdns_add_txt("s#", "1");
3221-
// feature flags (required if non-zero)
3222-
// bit 0 - supports HAP pairing. required for all HomeKit accessories
3223-
// bits 1-7 - reserved
3224-
homekit_mdns_add_txt("ff", "0");
3225-
// status flags
3226-
// bit 0 - not paired
3227-
// bit 1 - not configured to join WiFi
3228-
// bit 2 - problem detected on accessory
3229-
// bits 3-7 - reserved
3230-
homekit_mdns_add_txt("sf", "%d", (server->paired) ? 0 : 1);
3231-
// accessory category identifier
3232-
homekit_mdns_add_txt("ci", "%d", server->config->category);*/
3233-
32343203
if (server->config->setupId) {
32353204
DEBUG("Accessory Setup ID = %s", server->config->setupId);
32363205

@@ -3254,8 +3223,6 @@ void homekit_mdns_init(homekit_server_t *server) {
32543223
MDNS.announce();
32553224
MDNS.update();
32563225
homekit_mdns_started = true;
3257-
//INFO("MDNS ok! Open your \"Home\" app, click \"Add or Scan Accessory\""
3258-
// " and \"I Don't Have a Code\". \nThis Accessory will show on your iOS device.");
32593226
}
32603227

32613228
// Used to update the config_number ("c#" value of Bonjour)
@@ -3350,19 +3317,9 @@ void homekit_server_init(homekit_server_config_t *config) {
33503317
//homekit_server_task(server);
33513318
INFO("Starting server");
33523319

3353-
int r = homekit_storage_init();
3354-
if (r == 0) {
3355-
r = homekit_storage_load_accessory_id(server->accessory_id);
3356-
3357-
if (!r)
3358-
r = homekit_storage_load_accessory_key(&server->accessory_key);
3359-
}
3360-
3361-
if (r) {
3362-
if (r < 0) {
3363-
INFO("Resetting HomeKit storage");
3364-
homekit_storage_reset();
3365-
}
3320+
if (homekit_storage_init() != 0 ||
3321+
homekit_storage_load_accessory_id(server->accessory_id) != 0 ||
3322+
homekit_storage_load_accessory_key(&server->accessory_key) != 0) {
33663323

33673324
homekit_accessory_id_generate(server->accessory_id);
33683325
homekit_storage_save_accessory_id(server->accessory_id);

‎src/homekit_debug.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ typedef unsigned char byte;
1919
#define HOMEKIT_LOG_DEBUG 3
2020

2121
#ifndef HOMEKIT_LOG_LEVEL
22-
#define HOMEKIT_LOG_LEVEL HOMEKIT_NO_LOG
22+
#define HOMEKIT_LOG_LEVEL HOMEKIT_LOG_DEBUG
2323
#endif
2424

2525
#define HOMEKIT_PRINTF XPGM_PRINTF

‎src/storage.c

+62-47
Original file line numberDiff line numberDiff line change
@@ -78,59 +78,62 @@ extern uint32_t _SPIFFS_start; //See spiffs_api.h
7878

7979
#define STORAGE_DEBUG(message, ...) //printf("*** [Storage] %s: " message "\n", __func__, ##__VA_ARGS__)
8080

81-
const char magic1[] = "HAP";
81+
const char hap_magic[] = "HAP";
8282

8383
// TODO: figure out alignment issues
8484
typedef struct {
85-
char magic[sizeof(magic1)];
85+
char magic[sizeof(hap_magic)];
8686
byte permissions;
8787
char device_id[DEVICE_ID_SIZE];
8888
byte device_public_key[32];
8989

9090
byte _reserved[7]; // align record to be 80 bytes
9191
} pairing_data_t;
9292

93+
bool homekit_storage_magic_valid() {
94+
char magic_test[sizeof(hap_magic)];
95+
bzero(magic_test, sizeof(magic_test));
9396

94-
int homekit_storage_init() {
97+
if (!spiflash_read(MAGIC_ADDR, (byte *)magic_test, sizeof(magic_test))) {
98+
ERROR("Failed to read HomeKit storage magic");
99+
return false;
100+
}
101+
return (memcmp(magic_test, hap_magic, sizeof(hap_magic)) == 0);
102+
}
95103

96-
STORAGE_DEBUG("EEPROM max: %d B", SPI_FLASH_SEC_SIZE);//4096B
97-
STORAGE_DEBUG("Pairing_data size: %d ", (sizeof(pairing_data_t)));//80B
98-
STORAGE_DEBUG("MAX pairing count: %d ", MAX_PAIRINGS);//16
99-
STORAGE_DEBUG("_EEPROM_start: 0x%x (%u)",
100-
HOMEKIT_EEPROM_PHYS_ADDR, HOMEKIT_EEPROM_PHYS_ADDR);
101-
STORAGE_DEBUG("_SPIFFS_start: 0x%x (%u)",
102-
HOMEKIT_SPIFFS_PHYS_ADDR, HOMEKIT_SPIFFS_PHYS_ADDR);
104+
bool homekit_storage_set_magic() {
105+
if (!spiflash_write(MAGIC_ADDR, (byte *)hap_magic, sizeof(hap_magic))) {
106+
ERROR("Failed to write HomeKit storage magic");
107+
return false;
108+
}
109+
return true;
110+
}
103111

104-
char magic[sizeof(magic1)];
105-
memset(magic, 0, sizeof(magic));
112+
int homekit_storage_init() {
106113

107-
if (!spiflash_read(MAGIC_ADDR, (byte *)magic, sizeof(magic))) {
108-
ERROR("Failed to read HomeKit storage magic");
109-
}
114+
STORAGE_DEBUG("EEPROM max: %d B", SPI_FLASH_SEC_SIZE);//4096B
115+
STORAGE_DEBUG("Pairing_data size: %d ", (sizeof(pairing_data_t)));//80B
116+
STORAGE_DEBUG("MAX pairing count: %d ", MAX_PAIRINGS);//16
117+
STORAGE_DEBUG("_EEPROM_start: 0x%x (%u)",
118+
HOMEKIT_EEPROM_PHYS_ADDR, HOMEKIT_EEPROM_PHYS_ADDR);
119+
STORAGE_DEBUG("_SPIFFS_start: 0x%x (%u)",
120+
HOMEKIT_SPIFFS_PHYS_ADDR, HOMEKIT_SPIFFS_PHYS_ADDR);
110121

111-
if (strncmp(magic, magic1, sizeof(magic1))) {
122+
if (!homekit_storage_magic_valid()) {
112123
INFO("Formatting HomeKit storage at 0x%x", STORAGE_BASE_ADDR);
113-
if (!spiflash_erase_sector(STORAGE_BASE_ADDR)) {
124+
if (!spiflash_erase_sector(STORAGE_BASE_ADDR) || !homekit_storage_set_magic()) {
114125
ERROR("Failed to erase HomeKit storage");
115-
return -1;
126+
return -1; // Fail case
116127
}
117-
118-
strncpy(magic, magic1, sizeof(magic));
119-
if (!spiflash_write(MAGIC_ADDR, (byte *)magic, sizeof(magic))) {
120-
ERROR("Failed to write HomeKit storage magic");
121-
return -1;
122-
}
123-
124-
return 1;
128+
return 1; // Wasn't valid, is now
125129
}
126-
127-
return 0;
130+
return 0; // Was valid
128131
}
129132

130133

131134
int homekit_storage_reset() {
132-
byte blank[sizeof(magic1)];
133-
memset(blank, 0, sizeof(blank));
135+
byte blank[sizeof(hap_magic)];
136+
bzero(blank, sizeof(blank));
134137

135138
if (!spiflash_write(MAGIC_ADDR, blank, sizeof(blank))) {
136139
ERROR("Failed to reset HomeKit storage");
@@ -140,6 +143,18 @@ int homekit_storage_reset() {
140143
return homekit_storage_init();
141144
}
142145

146+
int homekit_storage_reset_pairing_data() {
147+
148+
byte blank[sizeof(pairing_data_t) * MAX_PAIRINGS];
149+
bzero(blank,sizeof(blank));
150+
151+
INFO("Formatting HomeKit storage at 0x%x", PAIRINGS_OFFSET);
152+
if (!spiflash_write(PAIRINGS_OFFSET, blank, sizeof(blank))) {
153+
ERROR("Failed to erase HomeKit pairing storage");
154+
return -1; // Fail case
155+
}
156+
return 0;
157+
}
143158

144159
void homekit_storage_save_accessory_id(const char *accessory_id) {
145160
if (!spiflash_write(ACCESSORY_ID_ADDR, (byte *)accessory_id, ACCESSORY_ID_SIZE)) {
@@ -209,7 +224,7 @@ bool homekit_storage_can_add_pairing() {
209224
pairing_data_t data;
210225
for (int i=0; i<MAX_PAIRINGS; i++) {
211226
spiflash_read(PAIRINGS_ADDR + sizeof(data)*i, (byte *)&data, sizeof(data));
212-
if (strncmp(data.magic, magic1, sizeof(magic1)))
227+
if (memcmp(data.magic, hap_magic, sizeof(hap_magic)))
213228
return true;
214229
}
215230
return false;
@@ -226,7 +241,7 @@ static int compact_data() {
226241
int next_pairing_idx = 0;
227242
for (int i=0; i<MAX_PAIRINGS; i++) {
228243
pairing_data_t *pairing_data = (pairing_data_t *)&data[PAIRINGS_OFFSET + sizeof(pairing_data_t)*i];
229-
if (!strncmp(pairing_data->magic, magic1, sizeof(magic1))) {
244+
if (!memcmp(pairing_data->magic, hap_magic, sizeof(hap_magic))) {
230245
if (i != next_pairing_idx) {
231246
memcpy(&data[PAIRINGS_ADDR + sizeof(pairing_data_t)*next_pairing_idx],
232247
pairing_data, sizeof(*pairing_data));
@@ -241,7 +256,7 @@ static int compact_data() {
241256
return 0;
242257
}
243258

244-
if (homekit_storage_reset()) {
259+
if (homekit_storage_reset_pairing_data()) {
245260
ERROR("Failed to compact HomeKit storage: error resetting flash");
246261
free(data);
247262
return -1;
@@ -291,10 +306,10 @@ int homekit_storage_add_pairing(const char *device_id, const ed25519_key *device
291306

292307
pairing_data_t data;
293308

294-
memset(&data, 0, sizeof(data));
295-
strncpy(data.magic, magic1, sizeof(data.magic));
309+
bzero(&data, sizeof(data));
310+
memcpy(data.magic, hap_magic, sizeof(data.magic));
296311
data.permissions = permissions;
297-
strncpy(data.device_id, device_id, sizeof(data.device_id));
312+
memcpy(data.device_id, device_id, sizeof(data.device_id));
298313
size_t device_public_key_size = sizeof(data.device_public_key);
299314
int r = crypto_ed25519_export_public_key(
300315
device_key, data.device_public_key, &device_public_key_size
@@ -317,10 +332,10 @@ int homekit_storage_update_pairing(const char *device_id, byte permissions) {
317332
pairing_data_t data;
318333
for (int i=0; i<MAX_PAIRINGS; i++) {
319334
spiflash_read(PAIRINGS_ADDR + sizeof(data)*i, (byte *)&data, sizeof(data));
320-
if (strncmp(data.magic, magic1, sizeof(data.magic)))
335+
if (memcmp(data.magic, hap_magic, sizeof(data.magic)))
321336
continue;
322337

323-
if (!strncmp(data.device_id, device_id, sizeof(data.device_id))) {
338+
if (!memcmp(data.device_id, device_id, sizeof(data.device_id))) {
324339
int next_block_idx = find_empty_block();
325340
if (next_block_idx == -1) {
326341
compact_data();
@@ -339,7 +354,7 @@ int homekit_storage_update_pairing(const char *device_id, byte permissions) {
339354
return -1;
340355
}
341356

342-
memset(&data, 0, sizeof(data));
357+
bzero(&data, sizeof(data));
343358
if (!spiflash_write(PAIRINGS_ADDR + sizeof(data)*i, (byte *)&data, sizeof(data))) {
344359
ERROR("Failed to update pairing: error erasing old record from HomeKit storage");
345360
return -2;
@@ -356,11 +371,11 @@ int homekit_storage_remove_pairing(const char *device_id) {
356371
pairing_data_t data;
357372
for (int i=0; i<MAX_PAIRINGS; i++) {
358373
spiflash_read(PAIRINGS_ADDR + sizeof(data)*i, (byte *)&data, sizeof(data));
359-
if (strncmp(data.magic, magic1, sizeof(data.magic)))
374+
if (memcmp(data.magic, hap_magic, sizeof(data.magic)))
360375
continue;
361376

362-
if (!strncmp(data.device_id, device_id, sizeof(data.device_id))) {
363-
memset(&data, 0, sizeof(data));
377+
if (!memcmp(data.device_id, device_id, sizeof(data.device_id))) {
378+
bzero(&data, sizeof(data));
364379
if (!spiflash_write(PAIRINGS_ADDR + sizeof(data)*i, (byte *)&data, sizeof(data))) {
365380
ERROR("Failed to remove pairing from HomeKit storage");
366381
return -2;
@@ -377,10 +392,10 @@ int homekit_storage_find_pairing(const char *device_id, pairing_t *pairing) {
377392
pairing_data_t data;
378393
for (int i=0; i<MAX_PAIRINGS; i++) {
379394
spiflash_read(PAIRINGS_ADDR + sizeof(data)*i, (byte *)&data, sizeof(data));
380-
if (strncmp(data.magic, magic1, sizeof(data.magic)))
395+
if (memcmp(data.magic, hap_magic, sizeof(data.magic)))
381396
continue;
382397

383-
if (!strncmp(data.device_id, device_id, sizeof(data.device_id))) {
398+
if (!memcmp(data.device_id, device_id, sizeof(data.device_id))) {
384399
crypto_ed25519_init(&pairing->device_key);
385400
int r = crypto_ed25519_import_public_key(&pairing->device_key, data.device_public_key, sizeof(data.device_public_key));
386401
if (r) {
@@ -389,7 +404,7 @@ int homekit_storage_find_pairing(const char *device_id, pairing_t *pairing) {
389404
}
390405

391406
pairing->id = i;
392-
strncpy(pairing->device_id, data.device_id, DEVICE_ID_SIZE);
407+
memcpy(pairing->device_id, data.device_id, DEVICE_ID_SIZE);
393408
pairing->device_id[DEVICE_ID_SIZE] = 0;
394409
pairing->permissions = data.permissions;
395410

@@ -416,7 +431,7 @@ int homekit_storage_next_pairing(pairing_iterator_t *it, pairing_t *pairing) {
416431
int id = it->idx++;
417432

418433
spiflash_read(PAIRINGS_ADDR + sizeof(data)*id, (byte *)&data, sizeof(data));
419-
if (!strncmp(data.magic, magic1, sizeof(data.magic))) {
434+
if (!memcmp(data.magic, hap_magic, sizeof(data.magic))) {
420435
crypto_ed25519_init(&pairing->device_key);
421436
int r = crypto_ed25519_import_public_key(&pairing->device_key, data.device_public_key, sizeof(data.device_public_key));
422437
if (r) {
@@ -425,7 +440,7 @@ int homekit_storage_next_pairing(pairing_iterator_t *it, pairing_t *pairing) {
425440
}
426441

427442
pairing->id = id;
428-
strncpy(pairing->device_id, data.device_id, DEVICE_ID_SIZE);
443+
memcpy(pairing->device_id, data.device_id, DEVICE_ID_SIZE);
429444
pairing->device_id[DEVICE_ID_SIZE] = 0;
430445
pairing->permissions = data.permissions;
431446

0 commit comments

Comments
 (0)