Linux Kernel(4.19) Hacks

rousalome.egloos.com

포토로그 Kernel Crash


통계 위젯 (화이트)

230224
1178
109352


리눅스 커널 기여(Contribution) II (1/3) -패치 작성 하기 Linux Kernel Contribution



패치 코드 작성 전 커널 코드 분석하기

이 포스팅을 올리는 주인공인 'Austin'은 리눅스 커널의 vmalloc 서브 시스템 내 __vmalloc_area_node() 함수 코드를 분석했습니다.

자, 그럼 소스 코드를 같이 볼까요? 특히 07~13번째 코드를 눈여겨봅시다. 
01 static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
02                 pgprot_t prot, int node)
{
03    struct page **pages;
04    unsigned int nr_pages, array_size, i;
...
05    area->nr_pages = nr_pages;
06    /* Please note that the recursion is strictly bounded. */
07    if (array_size > PAGE_SIZE) {
08        pages = __vmalloc_node(array_size, 1, nested_gfp|highmem_mask,
09               PAGE_KERNEL, node, area->caller);
10    } else {
11        pages = kmalloc_node(array_size, nested_gfp, node);
12    }
13    area->pages = pages;
14    if (!area->pages) {
15        remove_vm_area(area->addr);
16        kfree(area);
17        return NULL;
18   }
19    for (i = 0; i < area->nr_pages; i++) {
20        struct page *page;
...
21    }

소스 코드를 보고 다음과 같은 반문을 할 수 있습니다.

   *뭐가 문제지?

저도 커널 소스 코드를 분석할 때 이해하는데 급급할 때가 많습니다. 
하지만 커널 소스 코드를 조금 '비판적'으로 보면 가끔 논리적인 오류가 발견될 때가 있습니다.

자, 그러면 위 소스 코드의 문제점에 대해서 이야기를 해보려 합니다.
07    if (array_size > PAGE_SIZE) {
08        pages = __vmalloc_node(array_size, 1, nested_gfp|highmem_mask,
09               PAGE_KERNEL, node, area->caller);
10    } else {
11        pages = kmalloc_node(array_size, nested_gfp, node);
12    }

08번째와 11번째 줄 코드는 __vmalloc_node() 함수와 kmalloc_node() 함수를 호출해 페이지를 할당 받습니다. 그런데 메모리 부족으로 페이지가 부족할 때는 이 함수들은 NULL을 반환합니다. 

이번에 13~17번째 줄 코드를 보겠습니다.
13    area->pages = pages;
14    if (!area->pages) {
15        remove_vm_area(area->addr);
16        kfree(area);
17        return NULL;
18   }

다음 13번째 줄 코드를 보겠습니다.
페이지를 할당 받은 pages를 'area->pages' 필드에 저장합니다. page가 NULL일지도 모르는 상황인데도 'area->pages' 필드에 page를 저장합니다.

이번에는 14번째 줄 코드입니다.  'area->pages' 가 NULL인지 체크하는 동작입니다. 'area->pages' 가 NULL이면 '!area->pages' 구문은 TRUE이니 15번째 줄 코드를 실행해 'area->addr'와  'area' 주소를 해제(Free) 합니다. 

그래서 다음과 같은 아이디어가 생겼습니다.

   * 08번째와 11번째 줄 코드와 같이 __vmalloc_node() 함수와 kmalloc_node() 함수를 호출해 페이지를 할당 
    받은 다음 바로 page 포인터에 대한 NULL 체크를 하자.

  * 이 14~18번째 줄 예외 처리 코드 다음에 'area->pages = pages' 코드를 실행하게 하자.  

위에서 설명한 기준에 따라 소스 코드를 수정했습니다. 12~20번째 줄 코드가 수정한 코드입니다.
01 static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
02                 pgprot_t prot, int node)
{
03    struct page **pages;
04    unsigned int nr_pages, array_size, i;
...
05    /* Please note that the recursion is strictly bounded. */
06    if (array_size > PAGE_SIZE) {
07        pages = __vmalloc_node(array_size, 1, nested_gfp|highmem_mask,
08                PAGE_KERNEL, node, area->caller);
09    } else {
10        pages = kmalloc_node(array_size, nested_gfp, node);
11   }
12
13    if (!pages) {
14        remove_vm_area(area->addr);
15        kfree(area);
16        return NULL;
17    }
18
19    area->pages = pages;
20
21    for (i = 0; i < area->nr_pages; i++) {
22       struct page *page;
...
23   }    

이번에는 패치 형태로 소스 수정 내역을 확인해보겠습니다.   
root@raspberrypi:/home/pi/kernel_src/linux-next# git diff mm/vmalloc.c
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index e66e7ff..0471c78 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2416,13 +2416,15 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
        } else {
                pages = kmalloc_node(array_size, nested_gfp, node);
        }
-       area->pages = pages;
-       if (!area->pages) {
+
+       if (!pages) {
                remove_vm_area(area->addr);
                kfree(area);
                return NULL;
        }

+       area->pages = pages;
+
        for (i = 0; i < area->nr_pages; i++) {
                struct page *page;

커밋과 커밋 메시지 작성하기

코드를 수정했으니 이제 패치를 커밋으로 생성해볼까요?

‘git add 파일_이름’ 명령어를 입력해 패치를 인덱스에 등록 합니다.   
root@raspberrypi:/home/pi/kernel_src/linux-next# git add mm/vmalloc.c 

다음 명령어를 입력해 커밋 메시지와 함께 커밋 인덱스를 추가합니다. 
root@raspberrypi:/home/pi/kernel_src/linux-next# git commit -m "mm/vmalloc.c: move 'area->pages' after if statement"
 [master 09c14a5] mm/vmalloc.c: move 'area->pages' after if statement
 1 file changed, 4 insertions(+), 2 deletions(-)

위 명령어에서 “커밋 메시지”는 다음과 같습니다.
"mm/vmalloc.c: move 'area->pages' after if statement"

이번에는 다음 명령어를 입력해 커밋 메시지에 ‘Signed-off-by: 이메일 주소’와 추가 커밋 메시지를 추가합니다. 
root@raspberrypi:/home/pi/kernel_src/linux-next# git commit --amend -s 

mm/vmalloc.c: move 'area->pages' after if statement

Signed-off-by: Austin Kim <austindh.kim@gmail.com>

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
...
#
# Changes to be committed:
#   modified:   mm/vmalloc.c
#

'Signed-off-by'는 오픈 소스 라이센스 정책을 이해했으며 정책을 따르겠다는 의도로 추가하는 '도장'입니다. 만약 'Signed-off-by' 없이 패치 코드를 Maintainer에게 전달하면 'Signed-off-by'와 함께 다시 패치를 보내란 답장을 받을 확률이 높습니다.

이어서 'mm/vmalloc.c: move 'area->pages' after if statement' 제목 아랫 부분에 패치를 설명하는 메시지를 추가합니다.

01 mm/vmalloc.c: move 'area->pages' after if statement
02 
03 If !area->pages statement is true where memory allocation fails, area is
04 freed.
05 
06 In this case 'area->pages = pages' should not executed.  So move
07 'area->pages = pages' after if statement.
08
09 Signed-off-by: Austin Kim <austindh.kim@gmail.com>
10
 
커밋 메시지를 작성할 때는 패치의 내용을 명확하게 설명할 수 있는 문장을 써야 합니다. 여기서 주의해야 할 점은 패치의 내용을 너무 과장해 설명하면 안됩니다. 패치를 설명할 때 만약 ‘Fix Bug…", “Use-after-Free Fix” 등등의 표현을 쓰면 패치를 리뷰할 때 Maintainer나 다른 커널 개발자로부터 다음과 같은 질문을 받을 가능성이 높습니다.

   * 테스트 케이스는 무엇이냐?
   * 문제가 발생할 때 커널 로그를 알려줘라.

패치 리뷰를 할 때 이런 질문에 일일이 대답을 해줘야 하며 제대로 답변을 못하면 패치는 거절될 가능성이 높습니다.

이제 다음 명령어로 패치를 하나 생성해보겠습니다.
root@raspberrypi:/home/pi/kernel_src/linux-next# git format-patch -1
0001-mm-vmalloc.c-move-area-pages-after-if-statement.patch

‘git format-patch -1’는 최신 커밋 기준으로 1개 패치를 생성시키는 명령어입니다. 결과 다음 파일이 생성됐습니다. 
0001-mm-vmalloc.c-move-area-pages-after-if-statement.patch

이번에 위 파일을 열어 보겠습니다.
  1 From a7c761e9946132891664b2817984bf2466552130 Mon Sep 17 00:00:00 2001
  2 From: Austin Kim <austindh.kim@gmail.com>
  3 Date: Fri, 30 Aug 2019 12:57:16 2019 +0900 
  4 Subject: [PATCH] mm/vmalloc.c: move 'area->pages' after if statement
  5
  6 If !area->pages statement is true where memory allocation fails, area is
  7 freed.
  8
  9 In this case 'area->pages = pages' should not executed.  So move
 10 'area->pages = pages' after if statement.
 11
 12 Signed-off-by: Austin Kim <austindh.kim@gmail.com>
 13 ---
 14  mm/vmalloc.c | 6 ++++--
 15  1 file changed, 4 insertions(+), 2 deletions(-)
 16
 17 diff --git a/mm/vmalloc.c b/mm/vmalloc.c
 18 index e66e7ff..0471c78 100644
 19 --- a/mm/vmalloc.c
 20 +++ b/mm/vmalloc.c
 21 @@ -2416,13 +2416,15 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
 22     } else {
 23         pages = kmalloc_node(array_size, nested_gfp, node);
 24     }
 25 -   area->pages = pages;
 26 -   if (!area->pages) {
 27 +
 28 +   if (!pages) {
 29         remove_vm_area(area->addr);
 30         kfree(area);
 31         return NULL;
 32     }
 33
 34 +   area->pages = pages;
 35 +
 36     for (i = 0; i < area->nr_pages; i++) {
 37         struct page *page;
 38
 39 --
 40 2.6.2
 41

핑백

덧글

댓글 입력 영역