Skip to contentSkip to conversation

a race in pid generation that causes pids to be reused immediatelly

PATCH Cyril Hrubis 1 participant 1 message Dec 6, 2010 2 messages not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 449629110.2441291439746332.JavaMail.root@zmail06.collab.prod.int.phx2.redhat.com

A message that is not in the archive (sent off-list or not yet received): 1406083705.2491291440102995.JavaMail.root@zmail06.collab.prod.int.phx2.redhat.com

#2 lore raw

Hi!

Quoted 10 lines
Quoted 8 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+			if (lc > 0) {
+				printf("last_pid = %d\npid = %d\n", last_pid, pid);
+				distance = PidDistance(last_pid, pid);
+				if (distance == 0 || distance > 30000) {

Hmm are you sure that distance > 30000 is good thing to assert? Who says that two pids keeps at least 2768 distance and even then, who says that 32768 is maximal pid (see below)?

It was using a expert's advise here,
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5fdee8c4a5e1800489ce61963208f8cc55e42ea1

The problem here is that following code is just hacked together to reproduce some bug, creating robust testsuite is much more work. Perhaps asking the person who wrote the code about more details/help would be way to go.

Quoted 7 lines
Quoted 2 lines

Review

Quoted 1 line
(quoted diff) +1 -0
+	return 0;

Rather use tst_exit() here.

check() function is used in main() here,

TEST(check())
cleanup();

Quoted 6 lines

As you can see that the test has not finished yet after returned. It might also break existing LTP standard options like,

-c n Run n copies concurrently
-I x Execute test for x seconds

as TEST() seems expecting a non-void function.

Ah I see. So remove the TEST() macro and change check() to void.

Quoted 9 lines
Quoted 9 lines

Review

Quoted 7 lines
(quoted diff) +7 -0
+	/* capture signals */
+	tst_sig(FORK, DEF_HANDLER, cleanup);
+
+	/* Pause if that option was specified */
+	TEST_PAUSE;
+
+	tst_tmpdir();

Do you really need temporary directory to test fork() ?

Quoted 6 lines

Not really, but as mentioned earlier that those were copied from a standard LTP test template that I don't if some LTP internal things were using them. For example, LTP internal seems make use of tst_sig() and TEST_PAUSE in setup() for those standard options,

-p Pause for SIGUSR1 before starting
-P x Pause for x seconds between iterations

Therefore, I am concerning about remove those temporary directory creating might break other standard options and alike.

As said before, it just creates temporary directory.

Quoted 9 lines
Quoted 8 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+/* The distance mod 32768 between two pids, where the first pid is
+   expected to be smaller than the second. */
+int PidDistance(pid_t first, pid_t second) {
+	return (second + 32768 - first) % 32768;
+}

The comment doesn't hold as when second is smaller than first the negative value would be smaller than 32768.

Again this is what the expert said. What would be your advise here?

Ask the expert ;). Seriously kernel folks should help with writing tests, they are the only ones who trully know what to test anyway.

Quoted 7 lines
Quoted 2 lines

Moreover the maximal pid value may be changed by writing to "/proc/sys/kernel/pid_max", then this code is broken.

One thing I can do here is to backup/restore /proc/sys/kernel/pid_max and set a value explicitly to avoid ambiguity.

Quoted 2 lines

Also I'm not sure if the result of such computation couldn't overflow/underflow as int may not be the same size as pid_t.

Is this still an issue after set pid_max a value explicitly like 32768? Thanks.

I'm not sure if it's possible to lower the value, I haven't read the kernel code.

Signature · 13 lines
Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
What happens now with your Lotus Notes apps - do you make another costly 
upgrade, or settle for being marooned without product support? Time to move
off Lotus Notes and onto the cloud with Force.com, apps are easier to build,
use, and manage than apps on traditional platforms. Sign up for the Lotus 
Notes Migration Kit to learn more. http://p.sf.net/sfu/salesforce-d2d
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list