INTERNAL: Fix a clang static analyzer issue about getpeername failure

[ing-eoking]

🔗 Related Issue

• jam2in/arcus-works#562

⌨️ What I did

• clang static analyzer 에서 감지한 케이스 중 하나를 수정합니다.

  • getpeername 이 항상 주소값이 들어감을 보장해주지 않습니다.

    memcached.c:732:38: warning: Passed-by-value struct argument contains uninitialized data (e.g., field: 's_addr') [core.CallAndMessage]
    snprintf(c->client_ip, 16, "%s", inet_ntoa(addr.sin_addr));

    memcached.c:13620:20: warning: Passed-by-value struct argument contains uninitialized data (e.g., field: 's_addr') [core.CallAndMessage]
    if (strcmp(inet_ntoa(sin->sin_addr), ADMIN_CLIENT_IP) != 0 ||

• conn_new 에서는 memset을 통해 addr 변수를 초기화했습니다.

  • 아래와 같이 처리했으나 여전히 warning이 발생하여 memset을 사용했습니다.

    
    if (getpeername(c->sfd, (struct sockaddr*)&addr, &addrlen) != 0) {
    /* Fail Handler */
    } else {
    snprintf(c->client_ip, 16, "%s", inet_ntoa(addr.sin_addr));
    }

• conn_listening에서는 socket accept 과정에서 addr 값이 할당되므로 getpeername 을 제거했습니다.

[jhpark816]

아래와 같이 처리했으나 여전히 warning이 발생하여 memset을 사용했습니다.

if (getpeername(c->sfd, (struct sockaddr*)&addr, &addrlen) != 0) {
/* Fail Handler */
} else {
snprintf(c->client_ip, 16, "%s", inet_ntoa(addr.sin_addr));
}

어떤 warning 인지를 구체적으로 보여주세요. 완전히 동일한 warning이라면, 왜 sin_addr 필드가 uninitialized 상태가 된다고 생각하나요?

[ing-eoking]

@jhpark816

동일한 warning이 발생했습니다. 정확한 이유를 말해주진 않아 확실하지 않으며, 이에 대해 관련 문서에서는 확실하게 알수는 없었습니다. 저는 우선 getpeername이 Success 일 경우 addr이 채워지는 것으로 알고 있습니다. 하지만 이에 대해 찾아보았을 때는 극히 드문 케이스에서 보장되지 않는 경우도 있는 것 같습니다.

Unix 환경에서 FTP 프로토콜을 사용중이고, STDIN을 Socket 파일(sfd = 0)로 리다이렉션을 한 상황에서 getpeername을 호출 한 경우 주소 저장이 되지 않았다고 합니다. 링크 : https://kldp.org/node/101825

[jhpark816]

@ing-eoking

동일한 warning이 발생했습니다.

• Analyzer에서 보여주는 warning 자체를 한번 보여주시죠.
• Clang static analyzer는 수행 중의 분석이 아니라 코드에 대한 정적 분석이 아닌가요? warning 예상 원인은 수행 도중에 오류가 나는 것으로 설명하고 있어서요.
• 만약 수행 중의 분석이라면, 문제 시점의 init_state 인자 값도 출력해 주세요.

[ing-eoking]

@jhpark816

수정되었습니다.

Analyzer에서 보여주는 warning 자체를 한번 보여주시죠.

발생한 warning은 아래와 같습니다. 변경점 Link

memcached.c:732:42: warning: Passed-by-value struct argument contains uninitialized data (e.g., field: 's_addr') [core.CallAndMessage]
        snprintf(c->client_ip, 16, "%s", inet_ntoa(addr.sin_addr));

Clang static analyzer는 수행 중의 분석이 아니라 코드에 대한 정적 분석이 아닌가요? warning 예상 원인은 수행 도중에 오류가 나는 것으로 설명하고 있어서요.

정적 분석이며, CentOS7, MacOS 환경에서 구동했을 때 이 Warning에 대하여 실행 도중 문제가 발생한 적은 없습니다.

[jhpark816]

@ing-eoking 아래와 같이 getpeername() 수행이 실패한 경우에만 memset() 수행하는 코드는 어떤가요? 동일하게 analyzer warning 발생하나요?

    /* save client ip address in connection object */
    struct sockaddr_in addr;
    socklen_t addrlen = sizeof(addr);
    if (getpeername(c->sfd, (struct sockaddr*)&addr, &addrlen) != 0) {
        if (init_state == conn_new_cmd) {
            mc_logger->log(EXTENSION_LOG_WARNING, c,
                           "getpeername(fd=%d) has failed: %s\n",
                           c->sfd, strerror(errno));
        }
        memset(&addr, 0, sizeof(addr));
    }
    snprintf(c->client_ip, 16, "%s", inet_ntoa(addr.sin_addr));

[ing-eoking]

네. 동일한 에러가 발생했습니다.

memcached.c:733:38: warning: Passed-by-value struct argument contains uninitialized data (e.g., field: 's_addr') [core.CallAndMessage]
    snprintf(c->client_ip, 16, "%s", inet_ntoa(addr.sin_addr));

[ing-eoking]

localhost 로 연결한 경우

client_ip : 0.0.0.0

ip를 127.0.0.1로 지정한 경우

client_ip : 127.0.0.1

[jhpark816]

@ing-eoking

• localhost인 경우 "0.0.0.0" 출력되는 것은 예상 밖이네요.
• empty string 경우는 발생하지 않는군요.
• Local에서 캐시 장비의 ip를 주는 경우도 한번 해 봅시다.