Tracking Issue for `windows_process_extensions_raw_attribute`

[michaelvanstraten]

View all comments

Feature Gate: #![feature(windows_process_extensions_raw_attribute)]

This is a tracking issue for adding support to attach raw attributes for process creation on Windows using the raw_attribute() method.

Public API

pub trait CommandExt {
    unsafe fn raw_attribute<T: Copy + Send + Sync + 'static>(
        &mut self,
        attribute: usize,
        value: T,
    ) -> &mut process::Command;
}

Steps / History

• Add ability to spawn Windows process with Proc Thread Attributes | Take 2 #114848
• Abstract ProcThreadAttributeList into its own struct #123604
• Stabilization PR

Unresolved Questions

• Fix passing of raw pointers
• Creating safe interface for setting attributes

[ChrisDenton]

Now that there's an initial implementation on nightly people can start experimenting with it. One thing worth exploring is if there's a better API here.

Notably attribute lists can potentially be reused between calls. They are also optionally used for thread creation as well as creating a process (though this is not yet implemented). This suggests it might be helpful to have a ProcThreadAttributes struct, or similar.

The simplest thing would be something that just wraps InitializeProcThreadAttributeList, UpdateProcThreadAttribute and DeleteProcThreadAttributeList. Any such struct would however have limitations. You can only add new or overwrite existing attributes. This means, for example, you can't just append to a list. An alternative would be to use a generic map structure and only construct the attribute list at the last minute. This is much more flexible but also a little more wasteful. But process and thread creation is slow anyway so optimizing for it may not be worth it.

How ever it is implemented, it would also still be very unsafe. An attribute value can contain a struct which itself contains pointer to other data. It would be necessary for the user (or a library) to ensure that data outlives the attribute list. The same goes for other resources, e.g. ensuring HANDLEs aren't closed prematurely. We may want to expose safe wrappers for certain common attributes (e.g. PROC_THREAD_ATTRIBUTE_HANDLE_LIST or PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE).

[michaelvanstraten]

I currently see an issue with the new raw_attribute API that @TrPR124 and I implemented.

With the current interface, it is not possible to pass a pointer value as an attribute, or at least it seems so. This is required, for example, when trying to start a process attached to a pseudo-console, which is weird since this was the original intent of #88193.

Does anyone see any way the interface could be used to, for example, pass a pseudo-console handle to it?

[michaelvanstraten]

@ChrisDenton?

[ivansanchez-oss]

Any news on this topic?

[kazatsuyu]

With the current interface, it is not possible to pass a pointer value as an attribute, or at least it seems so.

I was experimenting with working with pseudo consoles in Windows and tried to use this feature, but found that it was not available for this reason.

Since UpdateProcThreadAttribute is basically designed to receive a pointer of some type, it is reasonable to keep it in a Box so that its value does not run out of life, and the existing API should remain as is.

However, it is not possible to handle cases where PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE is passed for attribute and a HPCON for value, so a new API may need to be added, for example as follows.

pub trait CommandExt {
    unsafe fn raw_attribute_ptr(
        &mut self,
        attribute: usize,
        ptr: PointerLikeType,
    ) -> &mut process::Command;
}

Of course, this is a more dangerous function than raw_attribute because its lifetime is unmanageable.

There are several points of question.

• What type should PointerLikeType be?
  • usize, isize, *const c_void or any other ideas?
• Is the corresponding value for cbSize in UpdateProcThreadAttribute necessary?
  • I am not sure if there are any use cases other than PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE that require this function.
  • Even if there is no other API than PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, it may be used if an API is provided. In this case, it is dangerous to pass a fixed value to cbSize.
  • The Creating a Pseudoconsole session tutorial passes sizeof(hpc), which is strange. In practice, it is unlikely that the size of HPCON will be needed, so the system may not check this value, but there is no guarantee

[michaelvanstraten]

@kazatsuyu do you know of any place where a raw pointer is used in a std function that interfaces with a system API?

[michaelvanstraten]

• The Creating a Pseudoconsole session tutorial passes sizeof(hpc), which is strange. In practice, it is unlikely that the size of HPCON will be needed, so the system may not check this value, but there is no guarantee

This also triped me up.

[michaelvanstraten]

• I am not sure if there are any use cases other than PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE that require this function.

For example, the GROUP_AFFINITY structure has a different size than HPCON which requires us to record the size dynamically.

Windows expects a thin pointer, so using *const T in the interface is misleading because somebody could pass a pointer with a different representation than *const c_void. Not using *const T then hinders us to look up the size of T at compile time.

We would have to accept a separate size parameter, which also could lead to a similar issue.

Optimally for me, we would have something like this:

pub trait CommandExt {
    unsafe fn raw_attribute<T: Sized, *const T: Repr<*const c_void>>(
        &mut self,
        attribute: usize,
        ptr: *const T,
    ) -> &mut process::Command {
        // record size_of::<T>() and the ptr as *const c_void
    }
}

[ivansanchez-oss]

Sorry for the newbie question, I just want to understand the problem a little more, why is it a problem that HPCON is wrapped in a Box in this scenario?

cmd.raw_attribute(PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, hpc.0);

Why is it not a problem in this other scenario that is in the documentation?

child_cmd.raw_attribute(PROC_THREAD_ATTRIBUTE_PARENT_PROCESS, parent.as_raw_handle() as isize);

[michaelvanstraten]

@Ivan-Sanchez-Diaz windows api is a bit weird here since it treats some values as literal pointers. The HPCON in the case of the pseudo console is treated as a pointer to some windows object so when wrapping it in a box and then passing a pointer to the value HPCON what we are doing is pointing to some memory in user space that holds a pointer to a windows object. So we actually need to treat HPCON as a pointer it self here hence why we have to change the interface.

Basically windows expects us to pass HPCON by value not by "reference".

Similar how it would be wrong to pass a &&str to a fn(&str).

In case of the parent process, what we are passing to UpdateProcThreadAttribute is a pointer to the parent process id, which when you think about it is similar when you think of HPCOM as a pointer to some object only we can not dereference HPOC because the value lives in kernel space.