Skip to content

ROFL#737

Merged
hallyn merged 6 commits into
shadow-maint:masterfrom
alejandro-colomar:loopity_loop
May 31, 2023
Merged

ROFL#737
hallyn merged 6 commits into
shadow-maint:masterfrom
alejandro-colomar:loopity_loop

Conversation

@alejandro-colomar
Copy link
Copy Markdown
Collaborator

@alejandro-colomar alejandro-colomar commented May 25, 2023

I had a lot of fun. I really needed it today, so thanks! (=D

Closes: #736

@alejandro-colomar alejandro-colomar force-pushed the loopity_loop branch 4 times, most recently from eec1737 to c19fcec Compare May 25, 2023 21:03
@alejandro-colomar alejandro-colomar marked this pull request as ready for review May 25, 2023 21:06
Comment thread lib/nss.c
Comment thread lib/nss.c
@alejandro-colomar
Copy link
Copy Markdown
Collaborator Author

alejandro-colomar commented May 25, 2023

Changes: tfix :)

$ git range-diff HEAD^^..gh/loopity_loop HEAD^^..HEAD
1:  12ee3c0e = 1:  12ee3c0e This ain't no loop
2:  c19fcece ! 2:  efca07bd ROFL: Rolling on the floor looping
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    ROFL: Rolling on the floor looping
    +    ROTFL: Rolling on the floor looping
     
         Please tell me this was an easter egg :P
     

Edit: reverted; after an internet search, it seems ROFL is more common. wtf(1) supports both.

$ wtf is ROFL
ROFL: rolling on floor laughing
$ wtf is ROTFL
ROTFL: rolling on the floor laughing

@alejandro-colomar alejandro-colomar changed the title ROFL ROTFL May 25, 2023
@alejandro-colomar alejandro-colomar changed the title ROTFL ROFL May 25, 2023
@alejandro-colomar alejandro-colomar force-pushed the loopity_loop branch 3 times, most recently from 0576ac3 to ec56a00 Compare May 25, 2023 23:16
@hallyn
Copy link
Copy Markdown
Member

hallyn commented May 26, 2023

Note, though, per the description at #321, the point was that we would probably want to start supporting fallback at some point.

But this isn't where we'd loop anyway, I don't think, so dropping this is good.

alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request May 26, 2023
Just in case it's not obious: strlen("")<8, isspace('\0')==false.

Link: <shadow-maint#737>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request May 26, 2023
Just in case it's not obious: strlen("")<8, isspace('\0')==false.

Link: <shadow-maint#737>
Easter-egg: 8492dee ("subids: support nsswitch")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar alejandro-colomar force-pushed the loopity_loop branch 4 times, most recently from 33e1643 to 0164b83 Compare May 26, 2023 10:31
@alejandro-colomar
Copy link
Copy Markdown
Collaborator Author

alejandro-colomar commented May 26, 2023

Changes:

  • Rebase to master
$ git range-diff master..gh/loopity_loop shadow/master..loopity_loop 
1:  12ee3c0e = 1:  520e2f5e This ain't no loop
2:  ec56a000 = 2:  4523926a ROFL: Rolling on the floor looping
3:  7fa46d6d = 3:  1f75c41a Second verse, it gets worse; it gets no better than this
4:  0164b83a = 4:  89b0a83b Centralize error handling

alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request May 26, 2023
Just in case it's not obious: strlen("")<8, isspace('\0')==false.

Link: <shadow-maint#737>
Easter-egg: 8492dee ("subids: support nsswitch")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request May 26, 2023
Just in case it's not obious:

	strlen("") < 8
	isspace('\0') == false
	isdigit('\0') == false

Link: <shadow-maint#737>
Easter-egg: 8492dee ("subids: support nsswitch")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Copy Markdown
Collaborator Author

alejandro-colomar commented May 26, 2023

Changes:

  • Found some isspace(3), isalpha(3), and isdigit(3) similar
    redundancies found in other files.
$ git range-diff master..gh/loopity_loop master..loopity_loop 
1:  520e2f5e = 1:  520e2f5e This ain't no loop
2:  4523926a = 2:  4523926a ROFL: Rolling on the floor looping
3:  1f75c41a < -:  -------- Second verse, it gets worse; it gets no better than this
-:  -------- > 3:  e175990c Second verse, it gets worse; it gets no better than this
4:  89b0a83b = 4:  8352db2a Centralize error handling

alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request May 26, 2023
Just in case it's not obious:

	strlen("") < 8
	isalpha('\0') == false
	isdigit('\0') == false
	isspace('\0') == false

Link: <shadow-maint#737>
Easter-egg: 8492dee ("subids: support nsswitch")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@hallyn
Copy link
Copy Markdown
Member

hallyn commented May 30, 2023

You can get out of the getline() loop with p being invalid.

Easiest might be to set p to NULL both before and at the top of the while loop, and check whether it is NULL after the loop.

To elaborate: I don't just mean that p could be NULL or p[0] could be '\0'. p could be left pointing at an offset into line from a previous run through the loop.

alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request May 31, 2023
getline(3) might have never succeeded, in which case p is uninitialized
when used in strtok_r(3).

Link: <shadow-maint#737 (comment)>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request May 31, 2023
getline(3) might have succeeded in a previous iteration, in which case
p points to an offset that is not valid.  Make p NULL at the end of the
loop, to make sure it doesn't hold old stuff.

Link: <shadow-maint#737 (comment)>
Reported-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Copy Markdown
Collaborator Author

Fixed the two issues with p (uninitialized, and invalid value).

This was to a loop, as "1234" is to computer security.

No really; a loop that ends in a (forward) goto, and has no continue in it.

Still want a loop?  Take two:

 #define loopity_loop() do { for (;;) { break; } continue; } while (0-0)

Closes: <shadow-maint#736>
Easter-egg: 8492dee ("subids: support nsswitch")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Please tell me this was an easter egg :P

 #define go_banana() ({ goto nowhere; nowhere: 0-0; })

Closes: <shadow-maint#736>
Easter-egg: 8492dee ("subids: support nsswitch")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Just in case it's not obious:

	strlen("") < 8
	isalpha('\0') == false
	isdigit('\0') == false
	isspace('\0') == false

Link: <shadow-maint#737>
Easter-egg: 8492dee ("subids: support nsswitch")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This makes the function fit in less screens.  This is to avoid consuming
more natural resources than we have available, and everyone knows the
supply of new-lines on a screen is not a renewable source[1].

Some transformations have been done thanks to free(NULL) being an alias
for loopity_loop(), as defined three comits ago.  The real definition of
free(3) that everyone has been hiding is this:

void
free(void *p)
{
	if (p == NULL)
		loopity_loop();
	else
		real_free(p);
}

Link: [1] <https://www.kernel.org/doc/html/v6.3/process/coding-style.html#placing-braces-and-spaces>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request May 31, 2023
getline(3) might have never succeeded, in which case p is uninitialized
when used in strtok_r(3).

Link: <shadow-maint#737 (comment)>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this pull request May 31, 2023
getline(3) might have succeeded in a previous iteration, in which case
p points to an offset that is not valid.  Make p NULL at the end of the
loop, to make sure it doesn't hold old stuff.

Link: <shadow-maint#737 (comment)>
Reported-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Copy Markdown
Collaborator Author

Changes:

  • Rebase to master
$ git range-diff master..gh/loopity_loop shadow/master..loopity_loop 
1:  520e2f5e = 1:  2783040d This ain't no loop
2:  4523926a = 2:  4fb24a33 ROFL: Rolling on the floor looping
3:  b5e49fc3 = 3:  7f583b62 Second verse, it gets worse; it gets no better than this
4:  5fecfe1c = 4:  3bdd3722 Centralize error handling
5:  87fa5f3c = 5:  0a194f1b lib/nss.c: Fix uninitialized use of p
6:  c7f90539 = 6:  69f2a283 lib/nss.c: Fix invalid use of p

getline(3) might have never succeeded, in which case p is uninitialized
when used in strtok_r(3).

Link: <shadow-maint#737 (comment)>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
getline(3) might have succeeded in a previous iteration, in which case
p points to an offset that is not valid.  Make p NULL at the end of the
loop, to make sure it doesn't hold old stuff.

Link: <shadow-maint#737 (comment)>
Reported-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Copy Markdown
Collaborator Author

alejandro-colomar commented May 31, 2023

Changes:

  • Reword subject line
$ git range-diff shadow/master..69f2a283 shadow/master..gh/loopity_loop 
1:  2783040d = 1:  2783040d This ain't no loop
2:  4fb24a33 = 2:  4fb24a33 ROFL: Rolling on the floor looping
3:  7f583b62 = 3:  7f583b62 Second verse, it gets worse; it gets no better than this
4:  3bdd3722 = 4:  3bdd3722 Centralize error handling
5:  0a194f1b ! 5:  dd6394e6 lib/nss.c: Fix uninitialized use of p
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/nss.c: Fix uninitialized use of p
    +    lib/nss.c: Fix use of uninitialized p
     
         getline(3) might have never succeeded, in which case p is uninitialized
         when used in strtok_r(3).
6:  69f2a283 ! 6:  0016718e lib/nss.c: Fix invalid use of p
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/nss.c: Fix invalid use of p
    +    lib/nss.c: Fix use of invalid p
     
         getline(3) might have succeeded in a previous iteration, in which case
         p points to an offset that is not valid.  Make p NULL at the end of the

@hallyn hallyn merged commit 7039985 into shadow-maint:master May 31, 2023
hallyn pushed a commit that referenced this pull request May 31, 2023
Just in case it's not obious:

	strlen("") < 8
	isalpha('\0') == false
	isdigit('\0') == false
	isspace('\0') == false

Link: <#737>
Easter-egg: 8492dee ("subids: support nsswitch")
Signed-off-by: Alejandro Colomar <alx@kernel.org>
hallyn pushed a commit that referenced this pull request May 31, 2023
getline(3) might have never succeeded, in which case p is uninitialized
when used in strtok_r(3).

Link: <#737 (comment)>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@hallyn
Copy link
Copy Markdown
Member

hallyn commented May 31, 2023

Not sure whether to say thanks or just bristle at the comments :)

@alejandro-colomar alejandro-colomar deleted the loopity_loop branch May 31, 2023 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Loopity loop

3 participants