espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.51k stars 7.26k forks source link

mdns_delegate_hostname_add crashes if hostname hasn't been set (IDFGH-6982) #8601

Closed tannewt closed 5 months ago

tannewt commented 2 years ago

I think @david-cermak has been working in mdns code most recently.

Environment

Problem Description

The IDF crashes with a LoadProhibited if mdns_delegate_hostname_add is called before mdns_hostname_set. This is caused by _hostname_is_ours not checking to see if _mdns_server->hostname is NULL. The top check should be:

    if (_mdns_server->hostname != NULL && strcasecmp(hostname, _mdns_server->hostname) == 0) {
        return true;
    }

Expected Behavior

It shouldn't crash.

Actual Behavior

It crashes with a LoadProhibited.

Steps to reproduce

Call mdns_delegate_hostname_add before mdns_hostname_set.

Debug Logs

Getting IP
Adding delegate
hostname copy 0x3fcf6c10 tannewt
address list copy 0x3fcf6c44
mdns actioho
            tnrme
hhosnnmm d0ne
000d0
Guru Meditation Error: Core  0 panic'ed (LoadProhibited). Exception was unhandled.

Core  0 register dump:
PC      : 0x400553cb  PS      : 0x00060630  A0      : 0x82097870  A1      : 0x3fcf7b80  
A2      : 0x3fcf6c10  A3      : 0x00000000  A4      : 0x00000000  A5      : 0x3fcf7af0  
A6      : 0x00000004  A7      : 0x3fca333c  A8      : 0x00000074  A9      : 0x00000000  
A10     : 0x00000000  A11     : 0x3c1584e1  A12     : 0x3ff191fd  A13     : 0x3fcf7ad1  
A14     : 0x00000008  A15     : 0xffffffff  SAR     : 0x00000000  EXCCAUSE: 0x0000001c  
EXCVADDR: 0x00000000  LBEG    : 0x40056f5c  LEND    : 0x40056f72  LCOUNT  : 0xffffffff  

Backtrace:0x400553c8:0x3fcf7b80 |<-CORRUPTED

(Garbled prints are from printing from both cores at once. It isn't the issue.)

chegewara commented 2 years ago

https://github.com/espressif/esp-idf/issues/8307 Probably not backported yet.

david-cermak commented 2 years ago

Hi @tannewt

Any particular reason why you've called add_delegated_host() before mdns_hostname_set() ? Are you trying to advertise or delegate services with no internal hostname? Or is it just the init part of your code?

tannewt commented 2 years ago

It wasn’t deliberate. I’m using the delegate to advertise a second name for the same ip. (One name is unique and one is a catch all.)

On Thu, Mar 31, 2022, at 10:58 PM, david-cermak wrote:

Hi @tannewt https://github.com/tannewt

Any particular reason why you've called add_delegated_host() before mdns_hostname_set() ? Are you trying to advertise or delegate services with no internal hostname? Or is it just the init part of your code?

— Reply to this email directly, view it on GitHub https://github.com/espressif/esp-idf/issues/8601#issuecomment-1085456208, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAM3KOFBRH7FQDCGBC5HG3VC2GBXANCNFSM5RAER3CQ. You are receiving this because you were mentioned.Message ID: @.***>

david-cermak commented 2 years ago

ok, thanks. I'll update the docs to make it clear that the own hostname is a prerequisite to advertizing services or delegating other names. it's just mentioned in the example:

https://github.com/espressif/esp-idf/blob/4350e6fef859b94f3efae6ceced751911c9d2cbc/examples/protocols/mdns/main/mdns_example_main.c#L41-L42

Also, as @chegewara (Thank you!) pointed out, some fixes from master haven't been backported to v4.4 yet. If you see any other issue with delagating services, please try to backport:

will be added to v4.4 soon.

tannewt commented 2 years ago

I think there is also a race condition in this code between the task setting _mdns_server->hostname and adding a service. It is possible to add a service with a NULL hostname if the mdns task hasn't processed the set hostname action yet. It then later crashes in a couple places that don't check that hostname for a service is not NULL.

david-cermak commented 5 months ago

closing as done via https://github.com/espressif/esp-protocols/pull/534