espressif / esp-lwip

Fork of lwIP (https://savannah.nongnu.org/projects/lwip/) with ESP-IDF specific patches
Other
85 stars 129 forks source link

Prevent crash when socket lock not initialized #11

Closed nebkat closed 4 years ago

nebkat commented 4 years ago

Crash occurs when running any socket related command (e.g. getsockopt) before a socket has been initialized for the first time.

freakyxue commented 4 years ago

hi @nebkat I don't think it is necessary to judge the value. because when allocate socket,this value has been judged. such as: In alloc_socket() function

if ESP_LWIP_LOCK

  if (!sockets[i].lock) {
    /* one time init and never free */
    if (sys_mutex_new(&sockets[i].lock) != ERR_OK) {
      return -1;
    }
  }

endif

nebkat commented 4 years ago

Hi @freakyxue,

The problem is that alloc_socket() only occurs once lwip_socket() has been called for the given socket[i].

In my code I am doing this (to find out about the currently open sockets in the system):

    for (int s = LWIP_SOCKET_OFFSET; s < LWIP_SOCKET_OFFSET + CONFIG_LWIP_MAX_SOCKETS; s++) {
        int err;

        int socktype;
        socklen_t socktype_len = sizeof(socktype);
        err = getsockopt(s, SOL_SOCKET, SO_TYPE, &socktype, &socktype_len);
        if (err < 0) continue;

        ... print info about socket ...
    }

If lwip_socket() hasn't been previously called for a given FD, the lock will not be initialized, and the program crashes on SYS_ARCH_PROTECT_SOCK(sock);, when it should simply return EBADF to getsockopt().

Perhaps more accurately it would be if (!sock->conn), let me know if you want me to update this.

freakyxue commented 4 years ago

hi @nebkat

from this point,it is really a bug. In tryget_socket_unconn function we can add the judgement:

tryget_socket_unconn(int fd) { struct lwip_sock *ret = tryget_socket_unconn_nouse(fd); if (ret != NULL && ret->conn != NULL) { if (!sock_inc_used(ret)) { return NULL; } } return ret; }