Page MenuHomeFreeBSD

shmfd: make shm_size a vm_ooffset_t
ClosedPublic

Authored by kevans on Jul 9 2020, 5:40 PM.

Details

Summary

On 32-bit platforms, this expands the shm_size to a 64-bit quantity and resolves a mismatch between the shmfd size and underlying vm_object size. The implementation did not account for this kind of mismatch.

Tentatively, I'd like to MFC it to stable/12, perhaps something like the following diff:

diff --git a/sys/kern/uipc_shm.c b/sys/kern/uipc_shm.c
index 700ee55bc99..d8a614afb41 100644
--- a/sys/kern/uipc_shm.c
+++ b/sys/kern/uipc_shm.c
@@ -531,6 +531,11 @@ shm_dotruncate(struct shmfd *shmfd, off_t length)
                object->charge += delta;
        }
        shmfd->shm_size = length;
+#if SIZE_MAX < UINT64_MAX
+       shmfd->shm_oldsize = length & 0xffffffff;
+#else
+       shmfd->shm_oldsize = length;
+#endif
        mtx_lock(&shm_timestamp_lock);
        vfs_timestamp(&shmfd->shm_ctime);
        shmfd->shm_mtime = shmfd->shm_ctime;
diff --git a/sys/sys/mman.h b/sys/sys/mman.h
index b2fad0e4757..aed759b5a8d 100644
--- a/sys/sys/mman.h
+++ b/sys/sys/mman.h
@@ -207,7 +207,7 @@ typedef     __size_t        size_t;
 struct file;

 struct shmfd {
-       size_t          shm_size;
+       size_t          shm_oldsize;
        vm_object_t     shm_object;
        int             shm_refs;
        uid_t           shm_uid;
@@ -230,6 +230,7 @@ struct shmfd {

        struct rangelock shm_rl;
        struct mtx      shm_mtx;
+       vm_ooffset_t    shm_size;
 };
 #endif

This would maintain some level of compatibility without breaking the layout too bad. New libprocstat would read garbage for the size on older 12 kernel core dumps (e.g. from fstat -M), but it's not clear to me that we strongly care about that.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kevans requested review of this revision.Jul 9 2020, 5:40 PM
kib accepted this revision.EditedJul 9 2020, 6:25 PM

Why do you need + shmfd->shm_oldsize = length & 0xffffffff; (for MFC) ? Simply assigning, possibly with explicit type conversion to silence a warning, should be good enough.

This revision is now accepted and ready to land.Jul 9 2020, 6:25 PM
In D25602#566434, @kib wrote:

Why do you need + shmfd->shm_oldsize = length & 0xffffffff; (for MFC) ? Simply assigning, possibly with explicit type conversion to silence a warning, should be good enough.

Sure, works for me. I was unsure if there's any implementation we care about that would truncate in some other fashion besides simply dropping the uppermost bits and err'd on the side of caution. I have no strong opinion, so I'll reduce that chunk to assignment with explicit cast to size_t. :)

In D25602#566434, @kib wrote:

Why do you need + shmfd->shm_oldsize = length & 0xffffffff; (for MFC) ? Simply assigning, possibly with explicit type conversion to silence a warning, should be good enough.

Sure, works for me. I was unsure if there's any implementation we care about that would truncate in some other fashion besides simply dropping the uppermost bits and err'd on the side of caution. I have no strong opinion, so I'll reduce that chunk to assignment with explicit cast to size_t. :)

Both size_t and vm_ooffset_t are unsigned. The C standard is explicit that conversion from wider unsigned type to narrow one takes the value mod max narrow, in other words it simply truncates upper bits for representation without padding. In other words, what you do with '&' happens automatically by cast.

In D25602#566440, @kib wrote:

Sure, works for me. I was unsure if there's any implementation we care about that would truncate in some other fashion besides simply dropping the uppermost bits and err'd on the side of caution. I have no strong opinion, so I'll reduce that chunk to assignment with explicit cast to size_t. :)

Both size_t and vm_ooffset_t are unsigned. The C standard is explicit that conversion from wider unsigned type to narrow one takes the value mod max narrow, in other words it simply truncates upper bits for representation without padding. In other words, what you do with '&' happens automatically by cast.

Oh, perfect, thanks. :-) Today, I learned.

This revision was automatically updated to reflect the committed changes.