netdev - Re: [RFC] connector: add group_exit_code and signal_flags fields to exit_proc

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]

Date:   Tue, 10 Apr 2018 19:01:13 +0300
From:   Stefan Strogin <sstrogin@...co.com>
To:     Evgeniy Polyakov <zbr@...emap.net>,
        "matthltc@...ibm.com" <matthltc@...ibm.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "xe-linux-external@...co.com" <xe-linux-external@...co.com>,
        Victor Kamensky <kamensky@...co.com>,
        Taras Kondratiuk <takondra@...co.com>,
        Ruslan Bilovol <rbilovol@...co.com>
Subject: Re: [RFC] connector: add group_exit_code and signal_flags fields to
 exit_proc_event

Hi Evgeniy,

On 09/04/18 02:49, Evgeniy Polyakov wrote:
> Hi everyone
> 
> Sorry for that late reply
> 
> 01.03.2018, 21:58, "Stefan Strogin" <sstrogin@...co.com>:
>> So I was thinking to add these two fields to union event_data:
>> task->signal->group_exit_code
>> task->signal->flags
>> This won't increase size of struct proc_event (because of comm_proc_event)
>> and shouldn't break backward compatibility for the user-space. But it will
>> add some useful information about what caused the process death.
>> What do you think, is it an acceptable approach?
> 
> As I saw in other discussion, doesn't it break userspace API, or you are sure that no sizes has been increased?
> You are using the same structure as used for plain signals and add group status there, how will userspace react,
> if it was compiled with older headers? What if it uses zero-field alignment, i.e. allocating exactly the size of structure with byte precision?
> 

Please ignore this RFC, I was wrong about the fields I need for the problem.
I have sent this patch: https://lkml.org/lkml/2018/3/29/531,
would be grateful for a review.

As for breaking UAPI and structure sizes, look:

> struct proc_event {
...
> 	union { /* must be last field of proc_event struct */
...
> 		struct comm_proc_event {
> 			__kernel_pid_t process_pid;
> 			__kernel_pid_t process_tgid;
> 			char           comm[16];
> 		} comm;
__kernel_pid_t is int that is always 4 bytes in Linux, then
sizeof(event_data.comm) == 24 on all platforms.
> 
> 		struct coredump_proc_event {
> 			__kernel_pid_t process_pid;
> 			__kernel_pid_t process_tgid;
> +			__kernel_pid_t parent_pid;
> +			__kernel_pid_t parent_tgid;
> 		} coredump;
sizeof(event_data.coredump) == 16
> 
> 		struct exit_proc_event {
> 			__kernel_pid_t process_pid;
> 			__kernel_pid_t process_tgid;
> 			__u32 exit_code, exit_signal;
> +			__kernel_pid_t parent_pid;
> +			__kernel_pid_t parent_tgid;
> 		} exit;
sizeof(event_data.exit) == 24
> 
> 	} event_data;
> };

Therefore, sizeof(event_data) is always = 24 - with old headers and new ones.
sizeof(struct proc_event) is the same as well.
Hence user-space software with old and new headers will allocate the same size.

If the user-space program somehow allocates space only for an internal structure,
e.g. for event_data.exit, I still don't see any problems if it allocates and
handles only first 16 bytes of the structure using old headers.

--
Stefan