Bug 246509

Summary: shmctl returns EIDRM when it should say EINVAL
Product: Red Hat Enterprise Linux 4 Reporter: Tom Lane <tgl>
Component: kernelAssignee: Anton Arapov <anton>
Status: CLOSED WONTFIX QA Contact: Martin Jenner <mjenner>
Severity: low Docs Contact:
Priority: low    
Version: 4.0CC: hhorak, jbaron, nobody
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-08-20 15:56:01 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Test program to exercise the bug none

Description Tom Lane 2007-07-02 18:55:47 UTC 
Description of problem:
The SysV shared memory code returns EIDRM for a collision of shm IDs.  I believe it should return
EINVAL instead.

Version-Release number of selected component (if applicable):
kernel-2.6.9-34.ELsmp

(Reproduced on that version and also on 2.6.20-1.2962.fc6; AFAICT this is an aboriginal
bug affecting all current Linux kernel versions)

How reproducible:
100% given idle system (no background creation/deletion of shmem segments)

Steps to Reproduce:
1. Create then drop 2 shmem segments, then create a third.
2. Try to shmctl(IPC_STAT) the two now-invalid shm IDs.
3. Note error codes returned.
 (See attached test program that does this)

Actual results:
One call gives EINVAL, one gives EIDRM due to collision with the third shmem segment.

Expected results:
Should both give EINVAL, I think; this is what the attached test program gives on every
other Unix I've tried it on.

Additional info:
I believe the problem is simply that shm_checkid() is returning the wrong error code: it ought
to fail with EINVAL not EIDRM.  There is no justification for EIDRM anywhere in shm.c, IMHO,
because the data structure simply does not store enough info to tell whether an ID is a "removed" one.

I suspect that the uses of EIDRM for SysV semaphores and message queues are equally bogus,
but have not looked into those modules carefully.
Comment 1 Tom Lane 2007-07-02 18:55:47 UTC 
Created attachment 158364 [details]
Test program to exercise the bug
Comment 2 Larry Woodman 2007-07-27 18:30:39 UTC 
Its actually inconsistant:

------------------------------------------------------------------------
[lwoodman@dhcp83-56 Desktop]$ ./shmctl-bug
id1 = 62357524 / 0x3b78014
id2 = 62390295 / 0x3b80017
id3 = 62423060 / 0x3b88014
shmctl(62357524 / 0x3b78014, IPC_STAT): ERROR: Identifier removed
shmctl(62390295 / 0x3b80017, IPC_STAT): ERROR: Invalid argument
------------------------------------------------------------------------

This is what the SHMCTL(2) manpage says for IPC_STAT:

EIDRM       is returned if shmid points to a removed identifier.

The attached program does:
1.) shmid1 = shmget(0x12345678, 1024 * 1024, IPC_CREAT | IPC_EXCL | 0600);
2.) shmid2 = shmget(0x12345678, 1024 * 1024, IPC_CREAT | IPC_EXCL | 0600);
3.) shmctl(shmid1, IPC_RMID, NULL);
4.) shmctl(shmid2, IPC_RMID, NULL);
5.) shmctl(shmid1, IPC_STAT, &buf);
6.) shmctl(shmid2, IPC_STAT, &buf);

The shmctl(shmid1, IPC_STAT, &buf) returns EIDRM while the 
shmctl(shmid2, IPC_STAT, &buf) returns EINVAL.

I'd say it should be returning EIDRM for both since the id was removed.


Larry Woodman
Comment 3 Larry Woodman 2007-07-27 18:40:29 UTC 
Actually, looking at this agian, I thing the program is wrong.  You
should be passing SHM_STAT and not IPC_STAT to shmctl.

[lwoodman@dhcp83-56 Desktop]$ ./shmctl-bug
id1 = 62750740 / 0x3bd8014
id2 = 62783511 / 0x3be0017
id3 = 62816276 / 0x3be8014
shmctl(62750740 / 0x3bd8014, IPC_STAT): ERROR: Invalid argument
shmctl(62783511 / 0x3be0017, IPC_STAT): ERROR: Invalid argument


[lwoodman@dhcp83-56 Desktop]$ diff shmctl-bug.c.orig shmctl-bug.c
69c69
<     if (shmctl(shmid1, IPC_STAT, &buf) < 0)
---
>     if (shmctl(shmid1, SHM_STAT, &buf) < 0)
75c75
<     if (shmctl(shmid2, IPC_STAT, &buf) < 0)
---
>     if (shmctl(shmid2, SHM_STAT, &buf) < 0)
Comment 4 Tom Lane 2007-07-28 03:38:45 UTC 
IPC_STAT is defined by the Single Unix Spec, SHM_STAT is not.  Telling me to use an unportable option to 
fix a lack of portable behavior is not an acceptable answer.

http://www.opengroup.org/onlinepubs/007908799/xsh/shmctl.html

Now that I look at it, the SUS mentions EINVAL but not EIDRM as a possible failure for shmctl(), so the 
current kernel behavior is not merely self-inconsistent but a flat violation of the spec.
Comment 5 Anton Arapov 2007-07-30 11:13:26 UTC 
  So, we are back to my questions... I'm fully support Tom. IMHO we should
follow specification. And in this case enough to change the behavior of the
shmctl/msgctl/semctl functions. But I don't know how this change reflects on the
kernel in a whole... this is one of the questions to gurus!
  The second, why this problem exists for a long time and never was discussed on
lkml?... I can't believe that no one had experience with this until now!

http://post-office.corp.redhat.com/archives/rhkernel-list/2007-July/msg00784.html
Comment 6 Anton Arapov 2007-08-20 15:51:12 UTC 
  Removing the EIDRM would break userspace applications currently depending on
it. The only way to write portable, between Linux and other *NIX clones,
application is to test for the error code by (EINVAL || EIDRM).