iamroot12CD / linux

linux kernel 4.1.6 for raspberrypi2
http://www.iamroot.org/
Other
6 stars 27 forks source link

atags_to_fdt()내의 memcount >= sizeof(mem_reg_property)/4)는 버그? #18

Closed minidump closed 8 years ago

minidump commented 8 years ago

atags_to_fdt.c의 atags_to_fdt()함수 내에서 for_each_tag() 루프안에 다음과 같은 코드가 있습니다.

                 } else if (atag->hdr.tag == ATAG_MEM) {
                         if (memcount >= sizeof(mem_reg_property)/4)
                                 continue;
                         if (!atag->u.mem.size)
                                 continue;

mem_reg_property는 uint32_t의 배열로 총 256바이트의 크기를 가지고 있습니다. 2 x 2 x 16(NR_BANKS) = 64 x 4(uint32_t) = 256 bytes

    /* In the case of 64 bits memory size, need to reserve 2 cells for
     * address and size for each bank */
    uint32_t mem_reg_property[2 * 2 * NR_BANKS];

그러면 mem_reg_property에 저장될 수 있는 reg프라퍼티 값의 최대 갯수는 32비트 메모리 사이즈의 경우 32개, 64비트 메모리 사이즈의 경우 16개가 됩니다. 그런데 루프안에서 첫 번째 if문을 보면 memcount >= sizeof(mem_reg_property)/4)라서 memcount가 64이상일 경우를 체크하고 있습니다.

그렇다면 아래 코드 블록에서 mem_reg_prop32나 mem_reg_prop64가 배열의 크기를 넘어버리는 경우가 발생할 수 있다라는게 스터디중 발견되었습니다.

            if (memsize == 2) {
                /* if memsize is 2, that means that
                 * each data needs 2 cells of 32 bits,
                 * so the data are 64 bits */
                uint64_t *mem_reg_prop64 =
                    (uint64_t *)mem_reg_property;
                mem_reg_prop64[memcount++] =
                    cpu_to_fdt64(atag->u.mem.start);
                mem_reg_prop64[memcount++] =
                    cpu_to_fdt64(atag->u.mem.size);
            } else {
                mem_reg_property[memcount++] =
                    cpu_to_fdt32(atag->u.mem.start);
                mem_reg_property[memcount++] =
                    cpu_to_fdt32(atag->u.mem.size);
            }

이게 버그가 맞을까요? 아니면 또 다른 이유가 있어서 이렇게 코드를 짰을까요?

norux commented 8 years ago

아, 위 내용이랑 상관없는 댓글인데 깃헙에서는 마크다운으로 코드 쓰실때 뒤에 언어 ex)c, ```java 를 딱 붙여주시면 코드하이라이팅까지 됩니다 :) 이 코멘트는 제가 수정해드렸어요.

minidump commented 8 years ago

@norux 네, 고맙습니다. 문서봐서 알고는 있는데 딱히 개의치 않고 사용하고 있었어요.

minidump commented 8 years ago

해당 이슈와 관련해서.. 해당 코드가 추가된 이력을 검색해봐서 찾은게 여기 http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/168218.html

그런데 이 코드는 리누스 토발즈의 리눅스 소스 https://github.com/torvalds/linux/commit/faefd550c45d8d314e8f260f21565320355c947f 에도 그대로 들어가 있는 내용. 즉, 해당 패치를 적용할 때 배열의 크기를 넘어버리는 예외처리는 수정되지 않은채 적용된것으로 보이네요. 이거 패치만들어서 담당 메인테이너에게 pull request 보내봐도 좋겠습니다. (스터디 진행용 이슈#17 참고)