[06:38] morning [07:21] good morning [07:26] zyga: hey [07:26] :-) [07:26] any crazy purchases? [07:27] * zyga starts the day with reviews [07:35] zyga: nope, not falling for the black friday bs with fake discounts around here [07:35] mborzecki: I got a real discount, I think [07:36] mborzecki: got a monitor at 2/3 the price (and I did check regular price as well as manufacturer recommended retail price0 [07:36] zyga: what display did you buy? [07:36] samsung space 27" 144Hz VA 2K panel [07:40] zyga: nice [08:01] morning [08:04] hey pstolowski [08:04] pstolowski: do you have that dock already? [08:04] mborzecki: https://github.com/snapcore/snapd/pull/7824#pullrequestreview-324890688 [08:04] PR #7824: snap/squashfs, osutil: verify files/dirs can be accessed by mksquashfs when building a snap [08:05] zyga: no, should arrive around Wed [08:08] zyga: thanks for the review [08:08] pstolowski: hey [08:29] Modeenv [08:29] why not ModeEnv [08:34] brb [08:40] hi, good morning [09:04] * zyga turned up heating [09:04] I have 15C in the office now [09:13] zyga: it's late atumn after all [09:16] and [09:17] the office has poor insulation :) [09:17] thinner walls, cold garage below [09:17] I would love to make it better one day, even if I have to insulate the celling downstairs [09:18] zyga: heh walk() does not follow symlinks, filepath.Walk("/snap/core/current"..), works differently that filepath.Walk("/snap/core/current/") [09:18] yeah :) [09:18] but it's just the outer initial symlink that matters to us [09:18] we don't want to follow symlinks inside, right? [09:20] hi, store team here. Store is currently down, it's being looked at. [09:20] * zyga hugs tomwardill [09:20] thank you and good luck [09:20] * tomwardill is mostly drinking coffee and watching people far more competent than I am, but I'll pass the sentiments on :) [09:21] zyga: anyways, refactored to use walk now, before i readdin'ing manually in order to faccessat, since that doesn't work bc of osx, might as well just use walk [09:21] mborzecki: macos has some innovation in the case of fstat-like functions [09:22] mborzecki: as well as in the case of "readdir" like functions [09:22] so the old syscalls are gone now [09:22] mborzecki: fstat has been replaced by something that is like statx but x10 more complex with lots of extra features and things one can ask about [09:22] mborzecki: and readdir is more like "search" now [09:22] tomwardill: from here it looked like a lot more than just the store was down :) [09:22] mborzecki: I think there's still the old readdir but it's deprecated and returns a subset of data now [09:23] Chipaca: PS4.5 is out [09:23] oouch! [09:23] tomwardill: is the internal irc on that? [09:23] yes [09:23] ah, nice [09:23] or at least, behind the same switch [09:23] ah [09:23] irc has just come back for me btw [09:23] will PS5 run on a xmas-lot of playstation 5 boxes? :) [09:23] i thought it ran on a 486 unde elmo's desk :-p [09:23] now powering AI/ML workloads ;) [09:24] haha [09:28] updated #7824 [09:28] PR #7824: snap/squashfs, osutil: verify files/dirs can be accessed by mksquashfs when building a snap [09:31] mborzecki: why not do those checks from snap/pack's loadAndValidate? [09:32] mborzecki: as part of ValidateContainer for ex [09:33] mborzecki: that way you still get the error in 'snap pack', but snapcraft gets to warn about it early [09:33] Chipaca: because it may be intentional to have the path set to 0000 or otherwise unreadable by the user [09:33] Chipaca: iirc that code checks that snap own meta and files are accessible [09:33] s/accessible/present/ [09:33] mborzecki: do you want to check the return value of Walk itself? [09:35] zyga: heh, clearly need another coffee [09:37] Chipaca: what i mean is that, current uid being unable to pack the snap doesn't mean it's invalid [09:37] mborzecki: one more question https://github.com/snapcore/snapd/pull/7824/files#r352494084 [09:37] PR #7824: snap/squashfs, osutil: verify files/dirs can be accessed by mksquashfs when building a snap [09:37] mborzecki: right [09:37] Chipaca: that's true but it also means the current uid won't be able to pack it :) [09:37] and that's worth checking [09:38] mborzecki: added two comments and resuming other reviews [09:38] brb [09:41] zyga: i'm all for using a smarted syscall than access, but the only alternative is trying to open that location, checking permission bits is too simple [09:41] mborzecki: but access is doing just permission check [09:42] it doesn't do anything else, does it? [09:42] I guess we could just open and try, that's portable and reliable [09:42] heh, so maybe we should just open [09:46] morning! we are noticing that some times it takes a lot of time to get a snap installed due to snapd wanting random numbers and the rng not being initialized yet [09:46] which is the rationale for snapd needing random numbers when installing a snap¿ [09:46] ? [09:46] abeato: good morning, is this on a vm? [09:46] abeato: that's unclear, I cannot think of any [09:46] no, on real HW [09:47] abeato: we generate some random numbers for snap cookies [09:47] perhaps that's that [09:47] on the Jetson Xavier, and on other devices on other prokects [09:48] is there a hardware random number generator on the xavier? [09:48] I wonder if it's just snaps that are affected [09:48] or generally software waiting and waiting [09:49] there is a trng, yes, but I think that due to a kernel bug the rng gets ~5 minutes to get initialized [09:50] so, on one hand this is a kernel problem, but on the other hand I do not think it makes much sense that snapd has to block on urandom to be initialized [09:50] the main issue is that we are seeing this on many deviced, not just one [09:50] abeato: right, I understand that [09:50] I think we investigated that once before [09:51] but it's lots of other parts of the stack that want entrop [09:51] *entropy [09:51] and us being super careful won't fix the general "it's stuck" feel [09:51] I think systemd is in the same boat [09:51] we don't depend on randomness in an unreasonable way, I think, we could do another analysis to check if anything new was added by accident [09:52] hm, well, systemd starts, not sure if there is much delay there [09:52] for FDE you would get things blocked too, sure [10:03] store update: most of the store should be back, SSO is still down [10:27] What is the best way to strace/ltrace or debug an application inside a snap? Sway starts, Xwayland and Xwayland runs xkbcomp... And/Or does a wayland snap need X support ? [10:29] sdhd-sascha: there's snap run --strace [10:29] but it's sometimes a little broken [10:29] and AFAIK we also have snap run --gdb [10:30] as for Xwayland -- I don't know [10:30] zyga: yes, i saw a file in the source with the name strace. But didn't have the time to inspect ... So it just ask [10:31] Well, sway depends on dmenu for launching application. It's X11. [10:31] Then the most GDK/GTK applications are not compiled with wayland or prefer X11. [10:32] I read on a debian changelog, that there are problems with the clipboard between X11 and wayland. So they prefer to start the application as X, if available. [10:33] Now i wonder, what the usecase of wayland inside a snap is. If kiosk with custom application, then no X11 is needed. But if it's a full desktop, then we need Xwayland support. [10:33] Weston is very nice here. It does not depend on external Xwayland but has similar problems with xkbcomp currently [10:35] I'm sorry I just don't know enough about the desktop stack to give useful advice [10:36] https://github.com/snapcore/snapd/pull/7129 needs a second review [10:36] zyga: it's ok. my opinion is, i will make it run. After that i need help to implement some plugs for common GUI-Frameworks [10:36] it's very old by now [10:36] PR #7129: userd: allow setting default-url-scheme-handler [10:49] more reviews :) [10:56] r-e-v-i-e-w-s :) [11:10] zyga: can you take a look at https://github.com/snapcore/snapd/pull/7743 ? [11:10] PR #7743: snap-bootstrap: force partition table operations [11:10] sure, just a moment [11:10] brb [11:19] reading a longer test [11:19] I should take a swap day [11:19] need to burn it [11:19] before xmas [11:40] mborzecki: looking now [11:46] mborzecki: hmm [11:46] mborzecki: somewhat worried [11:46] mborzecki: so the thing that we're operating on is mounted, right? [11:46] because we've booted [11:46] and now we're changing partitions b [11:46] are all partitions that we are changing unmounted? [11:49] zyga: afaiu this ties into the uc20 setup process, i'm slightly concerned about it but i suppose it has been thought through [11:49] I'm concerned and not equally optimistic [11:50] zyga: heh, did you look through the spec? [11:50] rapidly [11:50] I should read it again [12:30] re [12:30] hey cmatsuoka [12:31] zyga: hi [12:31] zyga: I'm also reading sfdisk.c [12:33] zyga: it seems that we could use --no-reread instead of --force, and read the PT afterwards (which we already do) [12:34] zyga: but if we could trigger the partition device node creation in a different way, then this second PT update wouldn't be necessary anymore [12:35] zyga: I'm completely for understanding exactly what's happening there because I feel things are a bit out of control there [12:36] we are in agreement [12:37] so one unknown here is: why partx is necessary to trigger node creation for newly created partitions? sfdisk itself and blockdev --rereadpt won't do that [12:41] cmatsuoka: I'm checking what --force does in practice [12:41] one thing is is doing is fdisk_device_is_used check gets ignored [12:41] let's see what that does [12:42] zyga: yeah, I'm exactly there now [12:43] hmm [12:43] one ioctl :) [12:43] let's see what that is [12:43] zyga: it's a bit disturbing that all those utilities seem to use a certain amount of guesswork ("it seems that the kernel...", "I think this...", "I don't know why, but...") [12:43] yeah [12:44] linux plumbing layer is full of traps [12:44] it's not nice in many ways [12:44] (not that others are) [12:44] heh [12:44] manual pages are silent [12:44] looking at the kernel [12:44] that's the only relief [12:44] at the end of the day [12:44] you just read and know [12:47] I'm curious to know what is ABBA deadlock now [12:47] humm, I'll verify this, but I'm suspecting the node creation trigger happens with BLKPG but not with BLKRRPART [12:47] in initramfs what is mounted on /dev? [12:48] devtmpfs I guess? not sure [12:48] do you know what populates it? [12:49] is it kernel itself or kernel + udev or udev from kernel events? [12:49] I don't know. I can check there but I'm not in bootable state right now [12:50] But I think we must see that in the new initramfs xnox just built [12:51] which may be different compared to the old ones [12:51] xnox: you there? [12:54] reading the kernel [12:54] I start to see what happens [12:55] zyga: the ABBA deadlock happens when two threads get two locks and then they try to get each other's locks [12:55] if I remember correctly [12:55] ah [12:55] I see [12:55] I thought it's a weird abba reference [12:55] it's just A B B A, as in thread names [12:55] :D [12:56] ah yes, threads A and B :D [13:00] zyga: hey wait wait, you said --force will skip the BLKRRPART ioctl? [13:01] (and then the partition table won't be updated anyway?) [13:04] hm, not exactly, but it seems to me that the ioctl() call is always failing and --force is just masking that [13:05] that would explain everything except why the ioctl call fails [13:05] yes [13:05] force is igoring that [13:05] the ioctl always runs [13:06] I'm reading the kernel side to see when it fails [13:06] (I also have a cold, sorry for the frequent interrupts when I'm away) [13:06] so I think it's failing because seed is mounted, and --force just makes it ignore the fact that the ioctl failed [13:07] so --no-reread is more appropriate here, it seems [13:07] what is the error we are getting in sfdisk again [13:08] oh boy [13:08] zyga: this one: https://github.com/karelzak/util-linux/blob/master/disk-utils/sfdisk.c#L1696 [13:08] it's only logged [13:08] yeah but are we getting EINVAL, E...what? [13:09] zyga: I don't know, I'll have to run it again to be sure [13:09] there's a way to get more output [13:09] there's a DBG macro [13:09] just need to figure out how to enable it [13:13] zyga: LIBFDISK_DEBUG=all ? [13:13] thank you! [13:17] ok, building an instrumented image [13:18] cmatsuoka: this is an interesting page https://unix.stackexchange.com/questions/141476/forced-reread-of-partition-table-difference-between-blkrrpart-and-blkpg-ioctl [13:19] zyga: we can check this in the kernel, but maybe BLKRRPART fails when partitions are mounted and BLKPG doesn't? [13:20] yeah [13:20] I think so [13:20] so partx and possibly partprobe would work while sfdisk and blockdev fail [13:20] that would explain everything [13:39] zyga: https://pasteboard.co/IJonVQV.png [13:39] thanks! [13:40] EBUSY [13:40] yep [13:40] just three places where that is returned [13:40] looking [13:41] or just a few [13:56] cmatsuoka: when we partition the disk, what's the state of the existing partitions [13:57] cmatsuoka: is the disk largely unpartitioned [13:57] cmatsuoka: and we append? [13:57] I read the code and I think the change is "safe" [13:57] zyga: we have the system-seed partition there, which is also our boot partition [13:57] in this sense [13:57] brb, see you at the standup [14:07] hey, sorry for this "noob" question. But where is the XDG_RUNTIME_DIR set on login ? I grep'ed the whole /etc. Took a look at /etc/pam.d/. [14:07] I wonder why it was set on "ssh" and "ttyX", but not on "sudo su" [14:08] cmatsuoka: https://www.youtube.com/watch?v=ytWz0qVvBZ0 [14:09] oh, i think it's inside systemd [14:12] sdhd-sascha: it's complex [14:12] sdhd-sascha: it's set by pam AFAIK but I don't know enough to tell you and point to a file where this happens [14:12] Chipaca: :D [14:13] zyga: currently i only need to trigger the environment, so i can call "weston-launch" as root [14:13] i could also set it manually.. [14:15] zyga: maybe i should look, howto increase the logging level of systemd ;-) [14:23] zyga: cmatsuoka: imo that delay is needed because partitions would show up/go away asynchronously [14:24] yeah [14:24] it's an event [14:24] we could instead look for them on disk [14:24] that would be less racy [14:24] if we know we expect /dev/xxx2 [14:24] zyga: cmatsuoka: and then udev is another async bit that should get some extra delay if you want to use /dev/disk/by-{name,label,uuid}.. [14:25] Chipaca: https://www.youtube.com/watch?v=dHoCeqlU2g8 (though it's mostly in Polish) [14:25] but it's fun ;) [14:25] mborzecki, zyga: yeah, I remember this node creation race that crashed the kernel back in the spike days [14:26] Chipaca: skip to 1;30 === ricab is now known as ricab|lunch [14:31] cmatsuoka: https://github.com/snapcore/snapd/pull/7743#issuecomment-560420462 [14:31] PR #7743: snap-bootstrap: force partition table operations [14:31] Chipaca: bah, no Diggy Diggy Hole t-shirts for me [14:31] Chipaca: hmm, maybe the 12-14 years size could fit? [14:33] cmatsuoka: the t-shirt link is a 404 from here so count yourself lucky (?) [14:34] Chipaca: if you go to the store main page and search from there you'll find a kids size t-shirt [14:34] cmatsuoka: i'm so sorry [14:34] * cmatsuoka sings Down and down into the deep, who knows what we'll find beneath? [14:36] * Chipaca updates cmatsuoka's race card to 'dwarf' [14:37] actually there's one in the video that looks a lot like our friend zygmunt [14:42] cmatsuoka: is my comment sensible? [14:43] cmatsuoka: note, alternatively --no-reread is okay as well, as it has the equivalent effect [14:43] off to pick up the kids [14:45] zyga: I think --no-reread would be a more conservative approach because it has a well-defined meaning and maybe what --force does could change in future versions of the utility. In practice, now, both would have the same effect [14:45] I agree [14:45] Ok, I'll prepare a patch with --no-reread and update the comments [14:46] Thanks! [14:46] thank you zyga for digging into this [14:47] mborzecki: thanks for looking at --explain, [14:48] mborzecki: the marker, did you mean to use some kind prefix for each line [14:48] mborzecki: or something else? [14:48] Chipaca: https://www.youtube.com/watch?v=34CZjsEI1yU === alan_g_ is now known as alan_g [14:53] * zyga switches gears to https://github.com/snapcore/snapd/pull/7815 [14:53] PR #7815: tests: reduce the complexity of the test-snapd-sh snap [15:04] cachio: I pushed a small tweak there [15:05] zyga, thanks [15:05] I'll take a look to that PR [15:09] Chipaca: :-D [15:10] * cachio lunch [15:10] cmatsuoka: mucho bettero [15:12] pstolowski, the hotplug test is failing here https://paste.ubuntu.com/p/VbRsMH3Sjx/ [15:12] pstolowski, not sure it is related to any change [15:13] done recently [15:13] but if fails twice and I can reproduce it [15:14] * ijohnson is all caught up on emails/backlog [15:14] cachio: weird, we haven't touched anything there. full log? [15:14] zyga: shall I review https://github.com/snapcore/snapd/pull/7825 ? looks like it's ready for review? [15:15] PR #7825: many: use transient scope for tracking apps and hooks [15:16] pstolowski, https://paste.ubuntu.com/p/fwm2MhW6bs/ [15:16] pstolowski, the log is huge [15:17] pstolowski, it appears /dev/ttyUSB0 rwk, [15:17] dinner is coming up, everyone's home now [15:17] and we are trying to match "/dev/ttyUSB0 rw," [15:17] cachio: I need to read only a small chunk of the diff before finishing my review but it's the most important one [15:17] I'll finish today for sure [15:17] zyga, thanks [15:18] cachio: aah, maybe we landed "+k" change, that would explain it [15:18] cachio: we planned to change serial port permissions [15:21] pstolowski, ahh [15:21] so we should update the test in that case right? [15:22] pstolowski, I can do it after lunch [15:22] pstolowski, does it make sense? [15:26] * cachio lunch again === ricab|lunch is now known as ricab [15:34] ijohnson: hey, you asked about '... (i.e. seccomp) and non-root owned "/"' [15:34] good morning jdstrand :-) [15:34] yes I did ask about that [15:35] do you want me to re-ask? [15:35] ijohnson: I see the question. you asked if I had time to chat. I didn't then but can now. what is up?> [15:35] ah right sorry I have forgotten what I sent [15:36] let me find the forum page for the full context [15:36] so here someone is trying to use snaps on azure pipelines: https://forum.snapcraft.io/t/permissions-problem-using-snapcraft-in-azure-pipelines/13258 [15:36] ijohnson: ok, I am going through irc backscroll first and not caaught up on email yet [15:36] jdstrand: what's weird is that on azure pipelines, "/" is not root-owned, and so snap-confine refuses to run [15:38] jdstrand: and looking through git history it seems that the reason snap-confine does the check for "/" and all elements of the path down to where we have seccomp compiled programs being root owned is to protect against someone putting bad bpf programs there and doing an escalation attack [15:39] jdstrand: so my question is if we can do anything about this case to loosen that requirement or enable some kind of flag to allow running snaps on azure pipelines like this [15:39] ijohnson: I've read the topic and you are right in why we are doing that. we are doing that for security reasons. I can't think of a legitimate reason why / is not owned by root. [15:40] yeah it's weird, but it does seem like a deliberate action, it doesn't seem like something someone would do on accident [15:40] ijohnson: I would like to understand why that is a legitimate use case. usually, there is a coding error somewhere in a maintainer script type thing that accidentally chowns / [15:41] ijohnson: chown vsts $TPYO/ [15:41] ijohnson: there have been USNs to fix CVEs in deb packaging for this type of thing [15:41] ah yes I could see that happening [15:42] let me see if I can find a place we could file an issue to get more context from azure [15:42] jdstrand: it would be so funny if that was the case [15:43] jdstrand: "Q: can you fix your software not to check / owner" [15:43] jdstrand: "A: can you apply this patch not to chown / from maintainer script" [15:43] ;) [15:43] cachio: yes, thank you [15:43] zyga: yeah. well, ufw I think was the first thing to do this checking in Ubuntu anyway, and it found a CVE in hplip :) [15:44] jdstrand: haha [15:44] jdstrand: so it *is* good that we ask for root password to install that printer ;) [15:44] cachio: i've just verified, we now have "rwk" on serial port [15:45] zyga: people said: "jdstrand> your check is too strict. please fix". I was like, uhm, why does hplip own /? You realize it can now change anything underneath it, right? [15:45] zyga: i think something like `<< snap explain start >>\n`, `<< snap explain end >>\n` would be enough [15:45] zyga: heh [15:45] mborzecki: that was not part of the initial design but I'll play with it [15:45] it's an experiment after all :) [15:45] zyga: iirc kernel has some ------[ cut here ] ------------- markers for backtraces [15:45] mborzecki: ah, I se [15:45] it would be just printed twice [15:45] that's nice [15:46] I'm semi-afk [15:46] zyga: otherwise my feeling is that the output of the actual app and the one from explain could be easily confused [15:46] Lucy is roaming around the office [15:46] jdstrand: zyga: well I tried looking and without an azure account the best they can do to talk to us is to ask on the stack exchange :-/ [15:56] ijohnson: https://forum.snapcraft.io/t/permissions-problem-using-snapcraft-in-azure-pipelines/13258/13 [15:58] ijohnson: I suspect one of the people asking for the change can use that when filing a bug with their azure account [15:59] jdstrand: ack yes that sounds fine, I did also ask on their stack exchange about it fwiw [16:00] cool, thanks [16:00] thanks jdstrand! [16:36] * Chipaca afk [16:40] ogra: thanks for the response re usb drive. I don't mind an sd card and am happy to do that, but it feels more correct to be able to boot off the usb directly. if that requires a blob from someone I don't know though, I prefer the approach you outlined :) [17:32] re [17:33] git push [17:34] git-clippy: where do you want to push today [17:35] :) [17:36] PR snapd#7827 opened: tests: apply change on permissions to serial port on hotplug test [17:37] +1 [17:37] thanks [17:38] zyga: ^ can you look at it as well, trivial test update [17:38] I am going to the dentist now [17:38] yeah [17:38] I'll be back in 40 minutes [17:38] jdstrand: hey,maybe consider escaping the _ on https://forum.snapcraft.io/t/mpris-name-for-media-player-snap/14371/7 [17:38] pstolowski, I'll try to merge it today [17:38] ijohnson was faster :) [17:38] :-) [17:38] ijohnson, pstolowski thanks guys [17:39] pstolowski: what happened that the permissions changed? [17:39] zyga: folks want to lock the serial-port, there are various forum topics about this [17:39] zyga: apparmor (kernel?) change at some point, jdstrand updated serial-port to grant locking permission as it was missing know, was implicit before [17:40] * ijohnson goes to look [17:40] aha [17:40] s/know/now/ [17:40] kernel side changed or our side changed? [17:40] the patch looks good , I'm just wondering why it wasn't checked earlier and caused something to fail [17:41] zyga: afair it was an apparmor or kernel bug [17:41] zyga: but not ours [17:41] pstolowski: but do you know if "our" side has changed? [17:41] because it is clearly measuring snapd files [17:41] so it must have come from snapd patch [17:41] so ... why didn't this change happen in sync then? [17:41] zyga: yes, we added +k to serial.port.go [17:41] see https://forum.snapcraft.io/t/hotplug-doesnt-allow-access-like-a-gadget-slot-does/14090 [17:42] was the test not executed? [17:42] zyga: because this spread test is run only on demand (nested execution) [17:42]