내 두 번째 리눅스 커널 기여

리눅스 커널 소스에 두 번째 기여를 한 기념으로 어떻게 기여하게 되었는지 그 과정을 공유한다. 이전에 비해 코드 수정이긴 하지만, 길이나 난이도 면에서는 간단한 편이다. 어쨋든 그 기여를 위해 어떤 판단을 했고, 어떻게 자료 조사를 했는지에 대한 경험을 공유하고자 한다.

어떻게 기여할 만한 것을 찾았는가?

iamroot 커널 스터디에 계속 참여하던 중, 커널에서 부팅 시 전달된 파라미터를 적용하는 과정을 분석하게 되었다. (start_kernel() 내 parse_early_params()~print_unknown_bootoptions() 및 에러 출력 부분)

여기에서 커널 파라미터를 처리하는 과정에서 예외 처리가 부족한 부분을 확인할 수 있었다.

커널 파라미터가 처리되는 흐름

bootloader 등을 통해 커널이 메모리로 로딩될 때, 커널에 초기 설정을 위한 인자를 전달할 수 있다. 이를 커널 파라미터라 부르도록 하겠다. (대표적으로 grub의 메뉴 설정을 보면 사용할 커널 이미지 경로 뒤에 ro, quiet, splash, root= 등이 작성되어있는데, 이 부분이 커널 파라미터다.)

커널 파라미터의 주요 처리는 앞에서 설명했듯 start_kernel()의 parse_early_params()~print_unknown_bootoptions() 부근에서 처리된다. 일부 커널 파라미터는 setup_arch() 내에서 먼저 처리되기도 한다. (대표적으로 ARM64가 setup_arch() 내에서 콘솔 관련 파라미터를 처리한다.) 그리고 이 파라미터가 커널에서 읽을 수 있게 준비되는 과정은(boot_command_line 변수에 저장) 각 아키텍쳐마다 다르겠디만, 아마도 setup_arch() 내에서 처리될 것으로 예상된다.

참고로 ARM64에서 커널 파라미터를 추출하는 과정은 start_kernel() => setup_arch() => setup_machine_fdt() => early_init_dt_scan() => early_init_dt_scan_nodes()에서 처리된다. 이 부분은 이번에 기여한 커밋과 직접적인 관계는 없으므로 다음에 설명하려 한다.

커널 파라미터가 처리되는 과정을 간략하게 요약하면 다음과 같다.

커널이 처리할 파라미터 핸들러는 struct kernel_param 혹은 struct obs_kernel_param 형태로 정의된다.
부팅시 전달받은 커널 파라미터를(boot_command_line 변수 등에 저장된 문자열) 스페이스 단위로 분리한다. (토큰화)
각 토큰이 기존 정의된 파라미터로 시작하는지 검사하고, 핸들러 함수를 수행하거나, 다음 파라미터 정의와 비교한다.
핸들러 함수가 정상 처리되었다면 다음 토큰에 대하여 2~3 과정을 반복한다.

커널 파라미터 선언 및 핸들러 등록

앞서 설명했듯, 파라미터 핸들러는 struct kernel_param 혹은 struct obs_kernel_param 형태로 정의된다. 각 구조체의 정의는 아래와 같다.

struct kernel_param_ops {
	/* How the ops should behave */
	unsigned int flags;
	/* Returns 0, or -errno.  arg is in kp->arg. */
	int (*set)(const char *val, const struct kernel_param *kp);
	/* Returns length written or -errno.  Buffer is 4k (ie. be short!) */
	int (*get)(char *buffer, const struct kernel_param *kp);
	/* Optional function to free kp->arg when module unloaded. */
	void (*free)(void *arg);
};

struct kernel_param {
	const char *name;
	struct module *mod;
	const struct kernel_param_ops *ops;
	const u16 perm;
	s8 level;
	u8 flags;
	union {
		void *arg;
		const struct kparam_string *str;
		const struct kparam_array *arr;
	};
};

struct obs_kernel_param {
	const char *str;
	int (*setup_func)(char *);
	int early;
};

각 구조체가 정의된 헤더의 파일 이름을 보면 struct kernel_param은 커널 모듈에 전달되는 파라미터를 정의하고, struct obs_kernel_param은 커널의 초기화 과정에서 처리될 파라미터만 정의하는 것으로 보인다. obs_는 obsolete의 약자로, 이 형태의 파라미터 핸들러 정의를 예전부터 사용했는데, 추후 도태시킬 생각인가 의심된다. (혹은 커널 초기화 과정에서만 사용되고 그 이후로는 사용되지 않으니 도태된다고 하는 것일 수도 있겠다.)

일단 현재 기여 내용과 관련있는 쪽은 struct obs_kernel_param 쪽인데, 이 부분을 좀 더 자세히 보자. 먼저 핸들러를 어떤 방식으로 등록하는지 관련된 코드는 아래와 같다.

/*
 * Only for really core code.  See moduleparam.h for the normal way.
 *
 * Force the alignment so the compiler doesn't space elements of the
 * obs_kernel_param "array" too far apart in .init.setup.
 */
#define __setup_param(str, unique_id, fn, early)			\
	static const char __setup_str_##unique_id[] __initconst		\
		__aligned(1) = str; 					\
	static struct obs_kernel_param __setup_##unique_id		\
		__used __section(".init.setup")				\
		__aligned(__alignof__(struct obs_kernel_param))		\
		= { __setup_str_##unique_id, fn, early }

/*
 * NOTE: __setup functions return values:
 * @fn returns 1 (or non-zero) if the option argument is "handled"
 * and returns 0 if the option argument is "not handled".
 */
#define __setup(str, fn)						\
	__setup_param(str, fn, fn, 0)

/*
 * NOTE: @fn is as per module_param, not __setup!
 * I.e., @fn returns 0 for no error or non-zero for error
 * (possibly @fn returns a -errno value, but it does not matter).
 * Emits warning if @fn returns non-zero.
 */
#define early_param(str, fn)						\
	__setup_param(str, fn, fn, 1)

#define INIT_SETUP(initsetup_align)					\
		. = ALIGN(initsetup_align);				\
		__setup_start = .;					\
		KEEP(*(.init.setup))					\
		__setup_end = .;

커널 파라미터 핸들러의 등록은 __setup() 혹은 early_param() 매크로를 통해 등록할 수 있다. 해당 매크로를 사용하면 struct obs_kernel_param을 .init.setup 섹션에 배치되게 하며, 파라미터의 이름은 별도의 상수에 선언하고, 멤버변수로 그 상수를 가리키게 설정한다.

커널 파라미터 핸들러의 등록, 순회 과정

그렇다면 실제 핸들러 함수를 등록하는 부분, 핸들러를 순회하는 과정을 코드로 확인해보자. (핸들러 함수 등록은 아래 예제 말고도 많이 있지만, 단편적인 것들만 다루겠다.)

/// usage of __setup() in /init/do_mounts.c

static int __init load_ramdisk(char *str)
{
	pr_warn("ignoring the deprecated load_ramdisk= option\n");
	return 1;
}
__setup("load_ramdisk=", load_ramdisk);

static int __init readonly(char *str)
{
	if (*str)
		return 0;
	root_mountflags |= MS_RDONLY;
	return 1;
}

static int __init readwrite(char *str)
{
	if (*str)
		return 0;
	root_mountflags &= ~MS_RDONLY;
	return 1;
}

__setup("ro", readonly);
__setup("rw", readwrite);

/// usage of early_param() in /drivers/tty/serial/earlycon.c

/* early_param wrapper for setup_earlycon() */
static int __init param_setup_earlycon(char *buf)
{
	int err;

	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
	if (!buf || !buf[0]) {
		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
			earlycon_acpi_spcr_enable = true;
			return 0;
		} else if (!buf) {
			return early_init_dt_scan_chosen_stdout();
		}
	}

	err = setup_earlycon(buf);
	if (err == -ENOENT || err == -EALREADY)
		return 0;
	return err;
}
early_param("earlycon", param_setup_earlycon);

extern const struct obs_kernel_param __setup_start[], __setup_end[];

static bool __init obsolete_checksetup(char *line)
{
	const struct obs_kernel_param *p;
	bool had_early_param = false;

	p = __setup_start;
	do {
		int n = strlen(p->str);
		if (parameqn(line, p->str, n)) {
			if (p->early) {
				/* Already done in parse_early_param?
				 * (Needs exact match on param part).
				 * Keep iterating, as we can have early
				 * params and __setups of same names 8( */
				if (line[n] == '\0' || line[n] == '=')
					had_early_param = true;
			} else if (!p->setup_func) {
				pr_warn("Parameter %s is obsolete, ignored\n",
					p->str);
				return true;
			} else if (p->setup_func(line + n))
				return true;
		}
		p++;
	} while (p < __setup_end);

	return had_early_param;
}

등록 과정의 예시를 보면, 매크로의 시작 부분에는 어떤 문자열로 시작하는 파라미터인지, (ex. "root=", "rootwait", "earlycon") 해당 파라미터를 받았을때 어떤 식으로 처리할지에 대한 함수를 정의하고 그 함수를 연결하고 있다.

순회하는 함수의 경우, 이전 .init.setup 섹션 부분의 시작과 끝 주소를 __setup_start, __setup_end에 할당한 것을 확인할 수 있다. 그리고 .init.setup의 시작 주소부터, 포인터로 하나씩 옮겨가며 반복문으로 기존 등록된 struct obs_kernel_param 구조체들을 순회한다. 먼저 parameqn()으로 현재 입력받은 커널 파라미터를 (정확히는 스페이스 단위로 분리한 토큰) 현재 순회하는 파라미터 핸들러와 비교해서 일치한다면, 핸들러 함수에 남은 문자열 부분을 전달한다.

호출된 핸들러 함수에서는 자체적으로 처리가 완료되면 1을, 자체적으로 처리가 안되면 0을 반환한다.

여기까지 분석한 부분이 iamroot 스터디를 진행하면서 알게 된 부분이자, 내가 기여한 부분을 이해하기 위한 기초 지식이다.

예외 처리가 필요한 커널 파라미터 핸들러

호출된 핸들러 함수에서는 자체적으로 처리가 완료되면 1을, 자체적으로 처리가 안되면 0을 반환한다.

이 문장이 기여한 부분의 핵심이다. 대표적으로 이런 상황을 일으키는 커널 파라미터 설명과 함께 확인해보자.

일반적으로 사용하는 커널 파라미터 중 ro, root=, rootwait가 있는데, 각각에 대한 커널 파라미터의 설명은 아래와 같다.

   ro              [KNL] Mount root device read-only on boot

   root=           [KNL] Root filesystem
                   See name_to_dev_t comment in init/do_mounts.c.

   rootwait        [KNL] Wait (indefinitely) for root device to show up.
                   Useful for devices that are detected asynchronously
                   (e.g. USB and MMC devices).

커널이 초기화를 끝내고, init 프로세스를 시작하기 전에 initramfs, initrd등을 사용하여 실제 파일 시스템 이전의 램 디스크를 사용하고, init에서 실질적으로 사용할 rootfs의 디스크로 교체하는 것으로 알고 있는데, 이 과정에 관련된 부분을 설정하는 커널 파라미터들이라고 볼 수 있다.

특히 해당 파라미터들을 예시로 드는 이유는, 모두 ro로 시작한다는 점이다. 순회하는 함수 obsolete_checksetup()의 내용을 보면, 각 등록된 커널 파라미터의 시작부분과 동일하다면(parameqn() 함수는 내부적으로 strncmp()나 memcmp()와 비슷하게 동작한다.) 핸들러 함수를 호출한다.

앞에서 핸들러를 등록하기 위해선 __setup() 매크로를 사용했는데, 해당 매크로는 struct obs_kernel_param이 지정된 섹션에(.init.setup) 위치하도록 할 뿐, 각각의 순서를 정하지는 못한다. 해당 매크로 선언 순서나, 오브젝트 파일들을 링킹하는 순서에 따라 반영될 뿐, 각 핸들러의 등록 순서를 강제할 수 있는 부분은 없다.

위 예시로 다루고 있는 ro, root=, rootwait의 핸들러 함수들을 다시 확인해보자.

static int __init readonly(char *str)
{
	if (*str)
		return 0;
	root_mountflags |= MS_RDONLY;
	return 1;
}

static int __init root_dev_setup(char *line)
{
	strlcpy(saved_root_name, line, sizeof(saved_root_name));
	return 1;
}

static int __init rootwait_setup(char *str)
{
	if (*str)
		return 0;
	root_wait = 1;
	return 1;
}

root=의 경우는 다른 커널 파라미터와 시작 부분이 겹치는 부분이 없어서 바로 1을 반환하지만, ro나 rootwait는 해당 파라미터 뒤에 어떤 글자라도 전달받았다면 해당 파라미터는 ro나 rootwait와 완전히 일치하지 않는 토큰을 전달받았다는 뜻이고, 아직 적절한 핸들러가 호출되지 않았을 수도 있으므로, 다른 핸들러를 돌려봐야 하므로 0을 반환한다.

모든 커널 파라미터 핸들러 검사

모든 핸들러들이 이런 식으로 파라미터 시작 부분이 겹칠 때, 예외처리를 잘 하는지 모든 __setup()을 검색해서, 겹치는 부분을 확인해봤다.

커널의 trace 기능에 tp_printk와 tp_printk_stop_on_boot 파라미터가 있는데, 아까 말한 예외 상황을 잘 잡지 못하는 것이 확인되었다.

static int __init set_tracepoint_printk(char *str)
{
	if ((strcmp(str, "=0") != 0 && strcmp(str, "=off") != 0))
		tracepoint_printk = 1;
	return 1;
}
__setup("tp_printk", set_tracepoint_printk);

static int __init set_tracepoint_printk_stop(char *str)
{
	tracepoint_printk_stop_on_boot = true;
	return 1;
}
__setup("tp_printk_stop_on_boot", set_tracepoint_printk_stop);

보면 tp_printk는 tp_printk_stop_on_boot의 시작부분과 일치한다. 하지만 tp_printk_stop_on_boot의 경우에 대한 예외 처리를 하고 있지 않다. 직접 해당 파라미터를 넣어 실행시켜 본 결과, tp_printk_stop_on_boot의 핸들러가 먼저 수행되어 버그를 확인할 수 없던 것으로 보인다. (현재 빌드 과정에서는 뒤에 선언된 부분이 먼저 할당되는 상황으로 보인다.) 해당 핸들러의 선언 순서만 바꿔줘도 예상대로 tp_printk_stop_on_boot가 tp_printk 안에서 처리가 완료되어 정상적으로 옵션이 처리되지 않는 것을 확인할 수 있었다.

패치 작성

아래와 같이 1차 패치를 작성했다.

trace: param: fix tp_printk option related with tp_printk_stop_on_boot

Kernel param "tp_printk_stop_on_boot" starts with "tp_printk" which is
the exact as the other kernel param "tp_printk".
In compile & build process, It may not guaranteed that
"tp_printk_stop_on_boot" always checked before "tp_printk".
(By swapping its __setup() macro order, it may not work as expected.)
Some kernel params which starts with other kernel params consider this
problem. See commit 745a600cf1a6 ("um: console: Ignore console= option")
or init/do_mounts.c:45 (setup function of "ro" kernel param)

Kernel param "tp_printk" can be handled with its value(0 or off) or
it can be handled without its value. (maybe it won't effect anything)
Fix setup function to ignore when the "tp_printk" becomes prefix of
other kernel param.

Signed-off-by: JaeSang Yoo <jsyoo5b@gmail.com>
---
 kernel/trace/trace.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c860f582b078..8611c1842a02 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -254,6 +254,8 @@ static int __init set_tracepoint_printk(char *str)
 {
        if ((strcmp(str, "=0") != 0 && strcmp(str, "=off") != 0))
                tracepoint_printk = 1;
+       if (tracepoint_printk || *str)
+               return 0;
        return 1;
 }
 __setup("tp_printk", set_tracepoint_printk);
--

So, "tp_printk=1" will return 0. That is not correct.

You want this to explicitly ignore the other parameter.

        if (*str == '_')
                return 0;

And it probably should be the first thing in the list. As we do not want to
enable tp_printk just because tp_printk_stop_on_boot is set. It requires
both: tp_printk tp_printk_stop_on_boot.

So the above check needs to be the first thing the function does.

내용을 확인해보면, tp_printk가 예외처리를 제대로 하지 않고 있으며, 이 부분을 ro 커널 파라미터 처리 방식이나 745a600cf1a6 패치처럼 예외처리를 해줘야 한다고 설명하고 아래와 같이 코드를 작성했다.

리뷰에서는 지금 나의 패치 방식으로는 tp_printk=1 같은 실제 설정이 변경되지는 않지만, 적절한 파라미터가 들어옴에도 0을 반환하는 방식이 되므로, 차라리 tp_printk 뒤에 _ 등이 들어온다면 다른 파라미터 핸들러일 수도 있으니 그 경우에만 0을 반환하도록 수정할 것을 권고받았다.

커밋 로그는 그대로 두고, 코드 수정사항만 추천대로 수정하고 보낸다는 것이, 너무 간단하고 신난 나머지 return 0에 세미콜론도 찍지 않고, 빌드로 확인도 안 해보고 바로 전송해버렸다. 제대로 테스트는 해봤냐고 혼나고, 이 예외 처리 코드에 대한 주석을 달 것을 추천받았다. 이번에는 빌드까지 제대로 확인해서 다시 패치를 전송했다.

최종적으로 패치가 적용되기 전에 리뷰어가 커밋 로그를 좀 더 정돈했고, mainline에 반영되었다.

tracing: Fix tp_printk option related with tp_printk_stop_on_boot

The kernel parameter "tp_printk_stop_on_boot" starts with "tp_printk" which is
the same as another kernel parameter "tp_printk". If "tp_printk" setup is
called before the "tp_printk_stop_on_boot", it will override the latter
and keep it from being set.

This is similar to other kernel parameter issues, such as:
  Commit 745a600cf1a6 ("um: console: Ignore console= option")
or init/do_mounts.c:45 (setup function of "ro" kernel param)

Fix it by checking for a "_" right after the "tp_printk" and if that
exists do not process the parameter.

Link: https://lkml.kernel.org/r/20220208195421.969326-1-jsyoo5b@gmail.com

Signed-off-by: JaeSang Yoo <jsyoo5b@gmail.com>
[ Fixed up change log and added space after if condition ]
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c860f582b078..7c2578efde26 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -252,6 +252,10 @@ __setup("trace_clock=", set_trace_boot_clock);

 static int __init set_tracepoint_printk(char *str)
 {
+       /* Ignore the "tp_printk_stop_on_boot" param */
+       if (*str == '_')
+               return 0;
+
        if ((strcmp(str, "=0") != 0 && strcmp(str, "=off") != 0))
                tracepoint_printk = 1;
        return 1;
--

stable에도 적용되다

이전 패치는 문서화 도구에 불과해서인지 mainline에만 적용되었는데, 이번 패치의 경우 버그 수정이라 그런지, 혹시 내 패치가 stable에도 적용되지 않을까? 하는 기대가 있었다.

다른 분을 통해 물어보니, stable의 경우 일반적인 패치의 경우 (특정 버전의 버그 수정이 아닌, mainline에 수정한 패치) 특별히 패치 반영을 요청할 필요 없이, 자체적으로 주요 패치를 반영한다고 한다.

이후 아래와 같이 여러 개의 메일을 받았다. 내용은 나의 패치가 현재 관리되고 있는 stable 버전들에도 적용된다는 것이다. (longterm으로 알려진 4.4, 4.9, 4.14, 4.19 등과, 가장 마지막 stable 버전인 5.16까지)

This is a note to let you know that I've just added the patch titled

    tracing: Fix tp_printk option related with tp_printk_stop_on_boot

to the 4.14-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     tracing-fix-tp_printk-option-related-with-tp_printk_.patch
and it can be found in the queue-4.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.

해당 메일 이전에도 [PATCH AUTOSEL ...] 같은 형태로 여러 메일이 왔었는데, 아마 CI 등을 통해 커밋 제목에 fix 등이 들어있다면 버그 수정으로 판단하여, 자동으로 stable에 반영되는 것 같다. 이 메일의 내용을 보면 알겠지만, 해당 패치가 stable에 적용되면 안되는 패치인 경우, 개별적으로 알려달라고 한다.