linux-kernel - Re: [RFC v2 PATCH 1/3] init: add sys-wrapper.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <AANLkTi=1mL_t-ado8yXntQFJR6P97JNdcTbXArXU0=ic@mail.gmail.com>
Date:	Mon, 30 Aug 2010 23:17:38 +0900
From:	Namhyung Kim <namhyung@...il.com>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Phillip Lougher <phillip@...gher.demon.co.uk>,
	Al Viro <viro@...iv.linux.org.uk>, linux-kernel@...r.kernel.org
Subject: Re: [RFC v2 PATCH 1/3] init: add sys-wrapper.h

On Mon, Aug 30, 2010 at 21:11, Arnd Bergmann <arnd@...db.de> wrote:
> On Sunday 29 August 2010, Namhyung Kim wrote:
>> +
>> +/* These macro are called just before/after actual syscalls. */
>> +#define KSYS_PREPARE                         \
>> +     mm_segment_t old_fs = get_fs();         \
>> +     set_fs(KERNEL_DS);
>> +
>> +#define KSYS_RESTORE                         \
>> +     set_fs(old_fs);
>
> These macros are not that nice, because they depend on context.
> I would probably open-code them in each function, or possibly
> use a single macro to combine it to something like
>
> #define kern_sys_call(call, ...)        \
> ({                                      \
>        mm_segment_t old_fs = get_fs(); \
>        long result;                    \
>        set_fs(KERNEL_DS);              \
>        result = call(__VA_ARGS__);     \
>        set_fs(old_fs);                 \
>        result;                         \
> })
>
> static inline int kern_sys_link(const char *oldname, const char *newname)
> {
>        return kern_sys_call(sys_link, (const char __user __force *)oldname,
>                             (const char __user __force *)newname);
> }
>

Cool. Will use it. :-)


>> +static inline int kern_sys_fchown(unsigned int fd, uid_t user, gid_t group)
>> +{
>> +     int ret;
>> +     KSYS_PREPARE;
>> +
>> +     ret = sys_fchown(fd, user, group);
>> +
>> +     KSYS_RESTORE;
>> +     return ret;
>> +}
>
> When there are no pointer arguments, there is no need to do set_fs
> tricks.
>

My intentions was it might be good, IMHO, if we have common setup/tear-down code
around actual syscall possibly extended in future. But now I think
it's a kind of over-
engineering so I'll discard it and follow your advice above.

Thanks.


-- 
Regards,
Namhyung Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/